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

Request metadata updates from peers #1700

Merged
merged 6 commits into from
Jul 25, 2024

Conversation

tillrohrmann
Copy link
Contributor

@tillrohrmann tillrohrmann commented Jul 7, 2024

This commit updates the network reactor to check the header of
the remote peer for newer metadata versions. If the remote peer
should have newer metadata, the reactor asks for it.

This fixes #1697.

This PR is based on #1699.

Comment on lines 114 to 127
pub fn version(&self, metadata_kind: MetadataKind) -> Version {
match metadata_kind {
MetadataKind::NodesConfiguration => self.nodes_config_version(),
MetadataKind::Schema => self.schema_version(),
MetadataKind::PartitionTable => self.partition_table_version(),
MetadataKind::Logs => self.logs_version(),
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

😍

crates/core/src/network/connection.rs Outdated Show resolved Hide resolved
crates/types/src/protobuf.rs Outdated Show resolved Hide resolved
crates/core/src/network/connection_manager/mod.rs Outdated Show resolved Hide resolved
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.

Great work @tillrohrmann as usual. Only a couple of small comments and we should be good to merge.

crates/core/src/network/connection.rs Outdated Show resolved Hide resolved
crates/types/src/config/common.rs Show resolved Hide resolved
@tillrohrmann
Copy link
Contributor Author

Thanks for the review @AhmedSoliman. I've addressed your comments and rebased onto the latest main to see if things pass.

This commit updates the network reactor to check the header of
the remote node for newer metadata versions. If the remote node
should have newer metadata, the reactor informs the MetadataFetcher
which tries to fetch the metadata updates from the remote node.
The MetadataFetcher makes sure that requests are regularly sent in
case that messages are being lost. Moreover, it ensures that only
one node that announced a given metadata version will be asked for
the update.

This fixes restatedev#1697.
Before, the MetadataMessageHandler panicked when being asked to send
the Schema metadata. Now, it will send the metadata.
The underlying assumption is that metadata won't change often.
Therefore, we assume that reading from Live<..> to obtain the
metadata versions is more efficient in the ConnectionSender.
This commit makes the connection sender stateful by remembering which
metadata version it has sent to a peer. Only new metadata versions,
except for the NodesConfiguration version, will be sent in order to
reduce a few bytes that need to be sent.
Urgency allows to specify how quickly the MetadataManager should react
to observed metadata versions. Currently, we support Normal and High.
Normal will pick up the observed metadata version on the next regular
update interval. High will immediately ask the MetadataManager to sync
the metadata from the metadata store.
@tillrohrmann tillrohrmann merged commit 54c47d0 into restatedev:main Jul 25, 2024
6 checks passed
@tillrohrmann tillrohrmann deleted the issues/1697 branch July 25, 2024 15:19
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.

Fetch metadata updates from peers
2 participants