[BEAM-9439] Return split instead of total backlog size in KinesisIO #11377
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The split backlog size can be over-estimated as it reports the size of the records across all shards (and also assumes that all shards in the split have the same progress). This can lead to unnecessary decisions to scale up the number of workers but will never fail to scale up when this is necessary. Also the watermark policy of the reader is user configurable and we don't have a control over it. The calculation of backlog bytes should always take into account the time that the record was inserted into Kinesis as this gives accurate results which are independent of current processing time or any other conditions that reader watermark can be using. Therefore a separate, fixed policy is used to track processed records for the backlog calculation.
Contributor
Author
Contributor
|
@sgraca Thank you for contribution! I'll take a look on this next week. |
Contributor
Author
|
Hi @aromanenko-dev Have you been able to make any progress with the review? |
Contributor
|
Run Java PreCommit |
Contributor
|
@sgraca Thank you for pinging me, sorry for delay. I'll try to review this asap. |
aromanenko-dev
approved these changes
May 7, 2020
sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/ShardRecordsIterator.java
Show resolved
Hide resolved
aromanenko-dev
approved these changes
May 11, 2020
Contributor
aromanenko-dev
left a comment
aromanenko-dev
left a comment
There was a problem hiding this comment.
LGTM, thank you @sgraca for working on this.
aromanenko-dev
changed the title
aromanenko-dev
changed the title
Contributor
Author
|
@aromanenko-dev Thank you for the review and the merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This merge request changes how backlog bytes are reported by KinesisReader.
Instead of total backlog bytes, which could result in Dataflow pipelines not scaling up,
split backlog is now reported.
The split backlog size can be over-estimated as it reports the size
of the records across all shards (and also assumes that all shards
in the split have the same progress).
This can lead to unnecessary decisions to scale up the number of workers
but will never fail to scale up when this is necessary.
Also the watermark policy of the reader is user configurable and we
don't have a control over it. The calculation of backlog bytes should
always take into account the time that the record was inserted into
Kinesis as this gives accurate results which are independent of
current processing time or any other conditions that reader watermark
can be using. Therefore a separate, fixed policy is used to track
processed records for the backlog calculation.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.