persist: add allow_compaction() operator#10029
Merged
aljoscha merged 1 commit intoJan 13, 2022
Merged
Conversation
Contributor
Author
|
I spun this off from #9981, so that we have the operator (and tests) isolated in one PR. |
1 task
danhhz
approved these changes
Jan 13, 2022
Contributor
danhhz
left a comment
danhhz
left a comment
There was a problem hiding this comment.
Very good, I like this much better than the -1!
| // need to be careful not to advance the compaction frontier up to the seal frontier, for | ||
| // which the input frontier is a proxy. | ||
| let mut candidate_compaction_frontier: Antichain<u64> = | ||
| Antichain::from_elem(timely::progress::Timestamp::minimum()); |
Contributor
There was a problem hiding this comment.
nit: add a use for timely::progress::Timestamp at the top of the file
| effective_input_frontier | ||
| ); | ||
|
|
||
| let future = write.allow_compaction(compaction_frontier.clone()); |
Contributor
There was a problem hiding this comment.
super nit: future is a common module name. I've seen petros use "fut" for this, which I like
| }); | ||
| } | ||
|
|
||
| // Swing through all pending futures and see if they're ready. Ready futures will |
Contributor
There was a problem hiding this comment.
Not for this PR, but really seems like a bunch of these operator impls have a pretty common pattern that we could maybe extract.
This operator will be used to allow compaction for persisted sources. The rustdoc and comments have all the information.
aljoscha
force-pushed
the
persist-add-allow-compaction-operator
branch
from
2358f08 to
5fd4d4f
Compare
Contributor
Author
|
tftr! I incorporated the nits. |
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
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 operator will be used to allow compaction for persisted sources.
The rustdoc and comments have all the information.
Checklist
The first commit is from #9938, and makes testing simpler. Without it I would have to first seal up to some frontier beyond my testing compaction frontiers, which would add chaff to the tests.