feat: Major classes for Spark continuous streaming #396

jiangmichaellll · 2020-12-03T04:48:19Z

This implements major classes for spark continuous streaming.

…nuous-processing

...k-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/MultiPartitionCommitter.java

...spark-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/PslContinuousReader.java

dpcollins-google · 2020-12-03T22:06:06Z

...spark-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/PslContinuousReader.java

+  private final MultiPartitionCommitter committer;
+  private PslSourceOffset startOffset;
+
+  public PslContinuousReader(PslDataSourceOptions options) {


this doesn't belong here, this is wiring code. Remove this constructor.

I would like to have this so I could keep code in PslDataSource super simple.

dpcollins-google · 2020-12-03T22:06:21Z

...spark-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/PslContinuousReader.java

+      AdminServiceClient adminServiceClient,
+      CursorServiceClient cursorServiceClient,
+      MultiPartitionCommitter committer) {
+    this.options = options;


don't take the options, take the things you need from it.

this is needed for lower level (PslContinuousInputPartition) for many params in the option(subPath, flowctrl, creds), prefer not to disassemble it early.

dpcollins-google · 2020-12-03T22:12:32Z

...spark-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/PslContinuousReader.java

+      assert PslSourceOffset.class.isAssignableFrom(start.get().getClass())
+          : "start offset is not assignable to PslSourceOffset.";
+      startOffset = (PslSourceOffset) start.get();
+      return;


Do these methods need to be thread safe?

No, they are called serially by the order of deserializeoffset (if writeahead log has any from previous query), setStartOffset, getStartOffset, planInputPartitions... get things done... mergeOffsets, commit.

...spark-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/PslContinuousReader.java

…tinuous-processing

jiangmichaellll · 2020-12-07T19:26:52Z

There is an extra issue in this PR. The buffering pull subscriber will lead to an unbounded message cache inside, thus not respecting the flow control. Please ignore this for this PR. I have another PR to address and make it bounded.

EDIT: #408 is the PR to make it bounded but let's finish this before I assign that.

dpcollins-google · 2020-12-08T21:33:50Z

...park-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/PslDataSourceOptions.java

+  @AutoValue.Builder
+  public abstract static class Builder {
+
+    public abstract Builder credentialsKey(String credentialsKey);


nit. these methods should be called "set"

dpcollins-google

Rebase this, fix the few comments, and then ping me back for approval

…continuous-processing

codecov · 2020-12-10T01:19:37Z

Codecov Report

Merging #396 (fabdebe) into master (63c34f0) will decrease coverage by 0.46%.
The diff coverage is 37.96%.

@@             Coverage Diff              @@
##             master     #396      +/-   ##
============================================
- Coverage     72.10%   71.63%   -0.47%     
- Complexity      845      871      +26     
============================================
  Files           158      163       +5     
  Lines          4399     4573     +174     
  Branches        222      226       +4     
============================================
+ Hits           3172     3276     +104     
- Misses         1104     1168      +64     
- Partials        123      129       +6

Impacted Files	Coverage Δ	Complexity Δ
.../pubsublite/spark/PslContinuousInputPartition.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...m/google/cloud/pubsublite/spark/PslDataSource.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...e/cloud/pubsublite/spark/PslDataSourceOptions.java	`11.11% <6.45%> (+11.11%)`	`2.00 <0.00> (+2.00)`
...le/cloud/pubsublite/spark/PslContinuousReader.java	`42.42% <42.42%> (ø)`	`10.00 <10.00> (?)`
...blite/spark/PslContinuousInputPartitionReader.java	`65.62% <65.62%> (ø)`	`6.00 <6.00> (?)`
.../pubsublite/spark/MultiPartitionCommitterImpl.java	`80.00% <80.00%> (ø)`	`6.00 <6.00> (?)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63c34f0...2950e08. Read the comment docs.

...k-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/MultiPartitionCommitter.java

...reaming/src/test/java/com/google/cloud/pubsublite/spark/MultiPartitionCommitterImplTest.java

jiangmichaellll added 6 commits November 30, 2020 23:03

PSL spark sql streaming utility classes

d88265c

update deps

ff546de

addressed comments

65dbca4

add back ignoreclasses

903fa6a

add used undeclared deps

cdab064

continuous streaming main things

b42e85e

jiangmichaellll requested review from a team as code owners December 3, 2020 04:48

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 3, 2020

product-auto-label bot added the api: pubsublite Issues related to the googleapis/java-pubsublite API. label Dec 3, 2020

jiangmichaellll assigned dpcollins-google Dec 3, 2020

address comments

0d7e816

jiangmichaellll unassigned dpcollins-google Dec 3, 2020

jiangmichaellll added 5 commits December 3, 2020 14:02

Merge branch 'jiangmichael-spark-utils' into jiangmichael-spark-conti…

1d6d4ba

…nuous-processing

address comments

356bf0b

Merge branch 'jiangmichael-spark-utils' into jiangmichael-spark-conti…

ceb47af

…nuous-processing

update

9ddfcac

update

db2a862

jiangmichaellll assigned dpcollins-google Dec 3, 2020

Base automatically changed from jiangmichael-spark-utils to master December 3, 2020 23:34

Merge branch 'master' into jiangmichael-spark-continuous-processing

985da4b

dpcollins-google requested changes Dec 4, 2020

View reviewed changes

jiangmichaellll added 5 commits December 3, 2020 21:20

partial address comments

0b094ef

offsets

7356b05

Merge branch 'jiangmichael-spark-offsets' into jiangmichael-spark-con…

0f33a8d

…tinuous-processing

partial address

a38304c

partial address

0d33492

jiangmichaellll changed the base branch from master to jiangmichael-spark-offsets December 4, 2020 07:04

remove

78c03af

jiangmichaellll added 4 commits December 4, 2020 02:11

update

6ee4a66

update

13fa44f

address comment

7a5517d

address comments

dba0e12

jiangmichaellll requested a review from dpcollins-google December 4, 2020 21:39

dpcollins-google reviewed Dec 8, 2020

View reviewed changes

dpcollins-google requested changes Dec 8, 2020

View reviewed changes

Base automatically changed from jiangmichael-spark-offsets to master December 9, 2020 06:22

jiangmichaellll added 2 commits December 9, 2020 19:51

Merge remote-tracking branch 'origin/master' into jiangmichael-spark-…

c569606

…continuous-processing

address comments

b4e5541

jiangmichaellll requested a review from dpcollins-google December 10, 2020 00:59

jiangmichaellll added 3 commits December 9, 2020 22:23

address comments

705da47

update

fe72e7b

update

a45bf59

yoshi-code-bot reviewed Dec 10, 2020

View reviewed changes

...k-sql-streaming/src/main/java/com/google/cloud/pubsublite/spark/MultiPartitionCommitter.java Outdated Show resolved Hide resolved

...reaming/src/test/java/com/google/cloud/pubsublite/spark/MultiPartitionCommitterImplTest.java Outdated Show resolved Hide resolved

jiangmichaellll added 2 commits December 10, 2020 14:54

update

fabdebe

update

2083b75

dpcollins-google approved these changes Dec 10, 2020

View reviewed changes

add more deps

2950e08

jiangmichaellll merged commit 0c0d928 into master Dec 10, 2020

jiangmichaellll deleted the jiangmichael-spark-continuous-processing branch December 10, 2020 22:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Major classes for Spark continuous streaming #396

feat: Major classes for Spark continuous streaming #396

jiangmichaellll commented Dec 3, 2020

dpcollins-google Dec 3, 2020

jiangmichaellll Dec 4, 2020

dpcollins-google Dec 3, 2020

jiangmichaellll Dec 4, 2020 •

edited

dpcollins-google Dec 3, 2020

jiangmichaellll Dec 4, 2020 •

edited

jiangmichaellll commented Dec 7, 2020 •

edited

dpcollins-google Dec 8, 2020

jiangmichaellll Dec 10, 2020

dpcollins-google left a comment

codecov bot commented Dec 10, 2020 •

edited

feat: Major classes for Spark continuous streaming #396

feat: Major classes for Spark continuous streaming #396

Conversation

jiangmichaellll commented Dec 3, 2020

dpcollins-google Dec 3, 2020

Choose a reason for hiding this comment

jiangmichaellll Dec 4, 2020

Choose a reason for hiding this comment

dpcollins-google Dec 3, 2020

Choose a reason for hiding this comment

jiangmichaellll Dec 4, 2020 • edited

Choose a reason for hiding this comment

dpcollins-google Dec 3, 2020

Choose a reason for hiding this comment

jiangmichaellll Dec 4, 2020 • edited

Choose a reason for hiding this comment

jiangmichaellll commented Dec 7, 2020 • edited

dpcollins-google Dec 8, 2020

Choose a reason for hiding this comment

jiangmichaellll Dec 10, 2020

Choose a reason for hiding this comment

dpcollins-google left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 10, 2020 • edited

Codecov Report

jiangmichaellll Dec 4, 2020 •

edited

jiangmichaellll Dec 4, 2020 •

edited

jiangmichaellll commented Dec 7, 2020 •

edited

codecov bot commented Dec 10, 2020 •

edited