coord,persist: teach coord about persisted streams that are involved in sources #9403
Conversation
This file contains 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
They are table-specific and we want to introduce a SourcePersistDetails.
…in sources Before, the coordinator would only know the "prefix" that was used to form the names of persistent streams that are involved in, say, the persistent Kafka source. Now, we keep a mapping from a _logical name_ to the physical stream name in the coordinator, for all persisted streams that are involved in a source. This also changes what persist information is sent to workers. Before, we would send a single persist name. Now, we send a new `SourcePersistDesc`, which contains the mapping of streams. In future commits, we will add more information to this. We need to do this to allow the coordinator to: - determine a common seal timestamp when restarting - negotiate/figure out if a requested as_of matches the compaction frontier/since of a persisted source - compact persisted streams when necessary We will do these additional changes in follow-up commits.
|
I think this breaks the builtin table, because we now render the |
|
Also: Happy to change all the mentions of persisted stream to persisted collection. |
danhhz
reviewed
Dec 2, 2021
| @@ -197,14 +197,18 @@ impl PersisterWithConfig { | |||
| } | |||
| } | |||
|
|
|||
| pub fn details(&self, id: GlobalId, pretty: &str) -> Result<Option<PersistDetails>, Error> { | |||
| pub fn details( | |||
There was a problem hiding this comment.
let's call this table_details now (ditto for table_details_from_name). maybe for the stuff in catalog.rs too?
|
Closing in favour of #9449 |
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Before, the coordinator would only know the "prefix" that was used to
form the names of persistent streams that are involved in, say, the
persistent Kafka source.
Now, we keep a mapping from a logical name to the physical stream name
in the coordinator, for all persisted streams that are involved in a
source.
This also changes what persist information is sent to workers. Before,
we would send a single persist name. Now, we send a new
SourcePersistDesc, which contains the mapping of streams. In futurecommits, we will add more information to this.
We need to do this to allow the coordinator to:
frontier/since of a persisted source
We will do these additional changes in follow-up commits.
Tips for reviewer
The contentious change is how I manually serialize (and deserialize) a mapping of names to the one
persist_namefield that we store in the catalog. I'm doing this in order to not break that, but could be convinced to do a more elaborate thing.Also, there are at least two
TODO:...s in the changes that are questions, please take a look at them.Checklist
user-facing behavior changes.