Replaces the consensus module with Bifrost #1253
Merged
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
tillrohrmann
force-pushed
the
issues/1216
branch
from
086070c
to
aa0be91
Compare
AhmedSoliman
approved these changes
Mar 4, 2024
There was a problem hiding this comment.
Very nicely executed. Very impressive work. Feel free to follow up on future PRs to unblock. Also, this needs to be rebased on #1256.
| header, | ||
| Command::InvokerEffect(invoker_output), | ||
| )) | ||
| .await?; |
There was a problem hiding this comment.
Same comment about handling bifrost failures and crashing the node.
tillrohrmann
force-pushed
the
issues/1216
branch
from
aa0be91
to
66b0d75
Compare
|
Rebased onto #1256. Will address the review feedback as a next step. Thanks a lot for it @AhmedSoliman. |
|
Rebasing the PR onto the latest |
tillrohrmann
force-pushed
the
issues/1216
branch
from
66b0d75
to
720077f
Compare
Moving the logic of command application into an inner struct separates a bit better the leadership acquisition logic and the command application.
tillrohrmann
force-pushed
the
issues/1216
branch
from
720077f
to
c31bd06
Compare
|
Rebased on latest master and fixed a few bugs which made the e2e tests fail. Now they should pass. I will now address your comments, Ahmed. |
tillrohrmann
force-pushed
the
issues/1216
branch
from
c31bd06
to
509ecbb
Compare
This commit removes the in-memory channel from which the PartitionProcessor receives messages from the Consensus module with Bifrost. This makes the PartitionProcessor independent of the Consensus module.
This commit replaces the in-memory channels with Bifrost in the Shuffle component.
By letting the AdminSerivce directly write messages to Bifrost, we can remove the Worker services.
By letting the ingress-dispatcher directly write to Bifrost, we no longer need the in-memory network channel.
With the introduction of Bifrost, we are no longer acking the reception of messages.
The dedup information is now sent as part of the destination field of the header. Therefore, these fields are no longer needed.
We are now sending dedup information as part of the destination field of the header. Therefore, the sequence_number field is no longer needed.
…oglet An Lsn::INVALID with base Lsn::OLDEST should translate into LogletOffset::INVALID instead of LogletOffset::OLDEST. The MemoryLoglet needs to add 1 to its index because the underlying array index is 0-based.
During Worker construction metadata() has not been properly initialized.
Until we go distributed there will be exactly one PartitionProcessor per log which can become leader when it starts.
…omponents This commit removes the with_bifrost function as it turned out to be more confusing than helping. Passing the Bifrost dependency explicitly to the components that need it turned out to be a lot simpler and easier to understand.
tillrohrmann
force-pushed
the
issues/1216
branch
from
509ecbb
to
888a86a
Compare
Since we are no longer requiring to flush writes to RocksDB, the existing batching mechanism might no longer needed. Moreover, it adds complexity which could be avoided until we assert that batching of commands has a measurable impact on Restate's performance. Until then and in order to make the future development a bit easier, we remove it.
tillrohrmann
force-pushed
the
issues/1216
branch
from
888a86a
to
d1d9f87
Compare
3 tasks
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.
This PR is currently based on #1250.
Important: With this PR we are temporarily losing durability because we are only relying on the
MemoryLoglet.I will add a more thorough description soon.