fix validate_progress in subgraph #327
Merged
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
It's unsafe to create capabilities based on the input frontier of the operator: a negative point-stamp change may be in flight, which would cause a race condition downstream
|
Seems legit! When you all have more time I'd be interested in the race, and the implications to the method (this seems to now catch more error cases, but it sounds like none of the implementors were screwing this up; is this more about writing down the correct statement about progress validation than changing the system's behavior?) |
utaal
changed the title
This was referenced Jun 23, 2020
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.
It's unsafe to create capabilities based on the input frontier of the operator: a negative incoming point-stamp change may be in flight, which would cause a race condition downstream.
I'm a little short on time right now, but I'd be happy to provide more details on the race condition. From preliminary testing, no operator violates this stricter requirement.
This was found as part of the verification effort at ETH, by @saradecova.