Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate local metadata store into the Node #1307

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

tillrohrmann
Copy link
Contributor

This commit integrates the local metadata store into the Node. It achieves it by introducing the MetadataStore role and requiring that the bootstrap node runs it. The role is the first started role. It runs the LocalMetadataStoreService. After starting the MetadataStore, the node tries to upsert the new NodeConfig with which it announces its NodeId, address and roles it runs.

This fixes #1296.

@tillrohrmann
Copy link
Contributor Author

This PR needs restatedev/e2e#292 to make the e2e tests working because the e2e tests depend on the config schema which was changed with this PR.

Copy link
Contributor

@AhmedSoliman AhmedSoliman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go, I'd be interested if we can drop the dependency on tokio-retry, but approving to unblock.

Cargo.toml Outdated Show resolved Hide resolved
crates/metadata-store/src/lib.rs Show resolved Hide resolved
crates/metadata-store/src/lib.rs Outdated Show resolved Hide resolved
crates/metadata-store/src/local/options.rs Outdated Show resolved Hide resolved
crates/metadata-store/src/local/options.rs Outdated Show resolved Hide resolved
crates/node/src/lib.rs Outdated Show resolved Hide resolved
crates/node/src/lib.rs Outdated Show resolved Hide resolved
Comment on lines +80 to +86
let (wipe_meta, wipe_worker, wipe_local_loglet, wipe_local_metadata_store) = match mode {
Some(WipeMode::Worker) => (false, true, true, false),
Some(WipeMode::Meta) => (true, false, false, false),
Some(WipeMode::LocalLoglet) => (false, false, true, false),
Some(WipeMode::LocalMetadataStore) => (false, false, false, true),
Some(WipeMode::All) => (true, true, true, true),
None => (false, false, false, false),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol 🤣

With retry_if it is now possible to check the error and decide
whether to continue retrying or not.
This commit integrates the local metadata store into the Node.
It achieves it by introducing the MetadataStore role and requiring
that the bootstrap node runs it. The role is the first started role.
It runs the LocalMetadataStoreService. After starting the MetadataStore,
the node tries to upsert the new NodeConfig with which it announces its
NodeId, address and roles it runs.

This fixes restatedev#1296.
@tillrohrmann tillrohrmann merged commit 56b4bb9 into restatedev:main Mar 26, 2024
5 checks passed
@tillrohrmann tillrohrmann deleted the issues/1296 branch March 26, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate metadata store with existing code base
2 participants