You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/05/31 15:56:28 UTC
[GitHub] [beam] damccorm opened a new pull request, #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
damccorm opened a new pull request, #17782:
URL: https://github.com/apache/beam/pull/17782
Right now, if you try to call TrySplit on an offsetrange restriction with a fraction of 0.0 and without first claiming work, it sets the primary restriction to {Start, Start-1}. This causes newSplitResult to panic - https://github.com/apache/beam/blob/ff39fcb5229b15140e41a61bd09f7d590730e93a/sdks/go/pkg/beam/core/runtime/exec/sdf.go#L859
This fixes the issue by never allowing End to be set less than Start. That in turn creates a possible issue with the IsDone function (since the restriction should be considered done, but it won't be until claimed reaches End), which this PR handles as well.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [x] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142317775
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck merged pull request #17782: [BEAM-14536] Handle 0.0 splits in offsetrange restriction
Posted by GitBox <gi...@apache.org>.
lostluck merged PR #17782:
URL: https://github.com/apache/beam/pull/17782
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142379667
R: @lostluck for final approval
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a diff in pull request #17782: [BEAM-14536] Handle 0.0 splits in offsetrange restriction
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17782:
URL: https://github.com/apache/beam/pull/17782#discussion_r886038681
##########
sdks/go/pkg/beam/core/runtime/exec/sdf.go:
##########
@@ -678,10 +678,6 @@ func (n *ProcessSizedElementsAndRestrictions) Checkpoint() ([]*FullValue, error)
return nil, addContext(err)
}
- if !n.rt.IsDone() {
- return nil, addContext(errors.Errorf("Primary restriction %#v is not done. Check that the RTracker's TrySplit() at fraction 0.0 returns a completed primary restriction", n.rt))
- }
Review Comment:
I had to remove this check because the new splitting logic ends up setting `rest.End = rest.claimed + 1`. Since the claimed work _should_ be complete, this means that the restriction is done, but that condition can't be checked here without adding new methods for users to implement (which doesn't seem worth it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] riteshghorse commented on a diff in pull request #17782: [BEAM-14536] Handle 0.0 splits in offsetrange restriction
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on code in PR #17782:
URL: https://github.com/apache/beam/pull/17782#discussion_r886844018
##########
sdks/go/pkg/beam/io/rtrackers/offsetrange/offsetrange.go:
##########
@@ -208,9 +210,11 @@ func (tracker *Tracker) GetProgress() (done, remaining float64) {
return
}
-// IsDone returns true if the most recent claimed element is past the end of the restriction.
+// IsDone returns true if the most recent claimed element is past the end of the restriction
+// or if the restriction represents no work to be done (aka the start of the restriction is
+// greater than or equal to the end).
func (tracker *Tracker) IsDone() bool {
- return tracker.err == nil && tracker.claimed >= tracker.rest.End
+ return tracker.err == nil && (tracker.claimed >= tracker.rest.End || tracker.rest.Start >= tracker.rest.End)
Review Comment:
Yeah, we should modify this as well. I prefer doing it `tracker.claimed+1 >= tracker.rest.End` as it is consistent with `GetProgress()`. I've done similar thing in GrowableTracker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a diff in pull request #17782: [BEAM-14536] Handle 0.0 splits in offsetrange restriction
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17782:
URL: https://github.com/apache/beam/pull/17782#discussion_r886882628
##########
sdks/go/pkg/beam/core/runtime/exec/sdf.go:
##########
@@ -678,10 +678,6 @@ func (n *ProcessSizedElementsAndRestrictions) Checkpoint() ([]*FullValue, error)
return nil, addContext(err)
}
- if !n.rt.IsDone() {
- return nil, addContext(errors.Errorf("Primary restriction %#v is not done. Check that the RTracker's TrySplit() at fraction 0.0 returns a completed primary restriction", n.rt))
- }
Review Comment:
I think you're right, added it back
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] riteshghorse commented on pull request #17782: [BEAM-14536] Handle 0.0 splits in offsetrange restriction
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142545619
I'm in favor of this change. This also aligns with our idea of offsetrange restriction being [start,end) (exclusive end).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142469943
I'm actually starting to question whether this is a case of me using this wrong. The runner _should_ never call split with 0.0, so we're pretty much just looking at self-checkpoints. In that case, is it _ever_ valid to self checkpoint without first calling `TryClaim` (@jrmccluskey). I had been trying to do something like:
```
i := rt.GetRestriction().(offsetrange.Restriction).Start
if !isDataAvailable(i) {
return sdf.ResumeProcessingIn(5 * time.Minute)
}
for rt.TryClaim(i) {
// ...
i++
if !isDataAvailable(i) {
return sdf.ResumeProcessingIn(5 * time.Minute)
}
}
```
But I'm not sure if that's an ok thing to do, or if I should be calling:
```
i := rt.GetRestriction().(offsetrange.Restriction).Start
for rt.TryClaim(i) {
if !isDataAvailable(i) {
return sdf.ResumeProcessingIn(5 * time.Minute)
}
// ...
i++
}
```
The latter actually _seems_ cleaner, but it also leads to questions about what we should be doing if we fail on the `TryClaim` call
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a diff in pull request #17782: [BEAM-14536] Handle 0.0 splits in offsetrange restriction
Posted by GitBox <gi...@apache.org>.
lostluck commented on code in PR #17782:
URL: https://github.com/apache/beam/pull/17782#discussion_r887134011
##########
sdks/go/pkg/beam/core/runtime/exec/sdf.go:
##########
@@ -678,10 +678,6 @@ func (n *ProcessSizedElementsAndRestrictions) Checkpoint() ([]*FullValue, error)
return nil, addContext(err)
}
- if !n.rt.IsDone() {
- return nil, addContext(errors.Errorf("Primary restriction %#v is not done. Check that the RTracker's TrySplit() at fraction 0.0 returns a completed primary restriction", n.rt))
- }
Review Comment:
Glad this was caught. This is basically the only place we can tell the user if they've implemented the SDF incorrectly in some fashion. While it's later than ideal (at execution, rather than construction time), a hard abort is preferable to other correctness or dataloss issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142479803
If we shouldn't ever be returning a process continuation until we call `TryClaim` then I'll update the error to specifically call that out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142375693
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @riteshghorse for label go.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142317763
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142317765
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142492381
The runner *should* never call TrySplit(0.0) but it's theoretically possible, so proper handling in the case it does should be a good precaution.
Returning a ProcessContinuation without trying to claim any work is definitely an anti-pattern as far as streaming SDFs are concerned. I'm confused by the distinction between TryClaim(i) and isDataAvailable(i) though, as in both cases once the position is claimed in the RTracker it won't be able to be claimed after the checkpoint.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] riteshghorse commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142327479
R: @lostluck
CC: @youngoli
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142366597
# [Codecov](https://codecov.io/gh/apache/beam/pull/17782?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17782](https://codecov.io/gh/apache/beam/pull/17782?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (57bb99c) into [master](https://codecov.io/gh/apache/beam/commit/4d22202fde91d5bc0b73369de39e4f4baa49739e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4d22202) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #17782 +/- ##
=======================================
Coverage 74.02% 74.02%
=======================================
Files 695 695
Lines 91872 91875 +3
=======================================
+ Hits 68007 68010 +3
Misses 22619 22619
Partials 1246 1246
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `50.49% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/17782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...o/pkg/beam/io/rtrackers/offsetrange/offsetrange.go](https://codecov.io/gh/apache/beam/pull/17782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9pby9ydHJhY2tlcnMvb2Zmc2V0cmFuZ2Uvb2Zmc2V0cmFuZ2UuZ28=) | `76.36% <100.00%> (+0.66%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17782?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/17782?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4d22202...57bb99c](https://codecov.io/gh/apache/beam/pull/17782?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a diff in pull request #17782: [BEAM-14536] Handle 0.0 splits in offsetrange restriction
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17782:
URL: https://github.com/apache/beam/pull/17782#discussion_r886879050
##########
sdks/go/pkg/beam/io/rtrackers/offsetrange/offsetrange.go:
##########
@@ -208,9 +210,11 @@ func (tracker *Tracker) GetProgress() (done, remaining float64) {
return
}
-// IsDone returns true if the most recent claimed element is past the end of the restriction.
+// IsDone returns true if the most recent claimed element is past the end of the restriction
+// or if the restriction represents no work to be done (aka the start of the restriction is
+// greater than or equal to the end).
func (tracker *Tracker) IsDone() bool {
- return tracker.err == nil && tracker.claimed >= tracker.rest.End
+ return tracker.err == nil && (tracker.claimed >= tracker.rest.End || tracker.rest.Start >= tracker.rest.End)
Review Comment:
Yeah, you're right - updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on a diff in pull request #17782: [BEAM-14536] Handle 0.0 splits in offsetrange restriction
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on code in PR #17782:
URL: https://github.com/apache/beam/pull/17782#discussion_r886825913
##########
sdks/go/pkg/beam/io/rtrackers/offsetrange/offsetrange.go:
##########
@@ -208,9 +210,11 @@ func (tracker *Tracker) GetProgress() (done, remaining float64) {
return
}
-// IsDone returns true if the most recent claimed element is past the end of the restriction.
+// IsDone returns true if the most recent claimed element is past the end of the restriction
+// or if the restriction represents no work to be done (aka the start of the restriction is
+// greater than or equal to the end).
func (tracker *Tracker) IsDone() bool {
- return tracker.err == nil && tracker.claimed >= tracker.rest.End
+ return tracker.err == nil && (tracker.claimed >= tracker.rest.End || tracker.rest.Start >= tracker.rest.End)
Review Comment:
Wouldn't the "done" case with claimed be `tracker.claimed >= (tracker.rest.End - 1)` since the range is [start, end)?
##########
sdks/go/pkg/beam/core/runtime/exec/sdf.go:
##########
@@ -678,10 +678,6 @@ func (n *ProcessSizedElementsAndRestrictions) Checkpoint() ([]*FullValue, error)
return nil, addContext(err)
}
- if !n.rt.IsDone() {
- return nil, addContext(errors.Errorf("Primary restriction %#v is not done. Check that the RTracker's TrySplit() at fraction 0.0 returns a completed primary restriction", n.rt))
- }
Review Comment:
I don't love removing this check since I think it has value in preventing a misconfigured RTracker from losing data. I also think that new behavior works with this if the `IsDone()` function for the offsetrange tracker is tweaked like I suggested
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on pull request #17782: [BEAM-14536] Make sure End is never larger than Start when splitting offsetrange restriction
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #17782:
URL: https://github.com/apache/beam/pull/17782#issuecomment-1142516057
I caught up with Jack briefly, and the takeaways were:
1) If you call TryClaim, you are saying that you are going to attempt to process the data (and it is your restriction's responsibility to handle that). This is consistent with the Java guidance (see https://beam.apache.org/documentation/programming-guide/#user-initiated-checkpoint).
2) Because of this, the residual restriction should _never_ contain a point that the primary restriction has already tried to claim.
3) Currently, for any 0.0 split point, the new primary is `[original.Start, original.Claimed)` and the residual is `[original.Claimed, original.End)`. This is wrong since the residual now contains `original.Claimed`.
I've updated my PR to fix this in a general way
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org