Rename serialized MetadataServerKind::Raft variant to replicated and MetadataStoreClientOptions into MetadataClientOptions #2632
Conversation
tillrohrmann
force-pushed
the
rename-raft-metadata-server
branch
from
7492101 to
4563f24
Compare
There was a problem hiding this comment.
Nice cleanup @tillrohrmann! I like "native metadata store", sounds good!
| .parse() | ||
| .expect("valid metadata store address")], | ||
| }, | ||
| connect_timeout: Duration::from_secs(3).into(), |
There was a problem hiding this comment.
Perhaps you have better observations about how this behaves in practice but is 3s perhaps much too high for a connect timeout?
There was a problem hiding this comment.
Something to observe and to adjust if this does not work out. We recently reduced it from 5s to 3s.
| @@ -603,40 +624,38 @@ pub enum MetadataStoreClient { | |||
| )] | |||
| // TODO(azmy): Remove this Shadow struct once we no longer support | |||
There was a problem hiding this comment.
What's preventing us from cleaning this up now?
There was a problem hiding this comment.
Right now we still need it for backward compatibility.
| @@ -806,6 +802,21 @@ pub struct CommonOptionsShadow { | |||
| initialization_timeout: humantime::Duration, | |||
| disable_telemetry: bool, | |||
|
|
|||
| metadata_client: MetadataClientOptions, | |||
| // todo drop in version 1.3 | |||
There was a problem hiding this comment.
I feel like we should have a convention for such things that we can easily grep for 😅 todo(release:1.3.0): ...?
There was a problem hiding this comment.
Yeah, or an attribute that would automatically generate a warning if it is still being used #[deprecated(1.2.0, 1.3.0)]. I'll leave this for a follow-up.
tillrohrmann
force-pushed
the
rename-raft-metadata-server
branch
2 times, most recently
from
8f5bc5c to
d37e6de
Compare
With this commit, the default way to configure the Raft based metadata server is to configure metadata-server.type = "replicated".
This commit also moves some of the metadata client specific options back
under the [metadata-client] key. It tries to achieve backwards compatibility
with the previous configuration names. The metadata client used with the local
and the replicated metadata server is now called "native" ("embedded" still works
as an alias).
tillrohrmann
force-pushed
the
rename-raft-metadata-server
branch
from
d37e6de to
7df62da
Compare
Rename serialized MetadataServerKind::Raft variant to replicated
With this commit, the default way to configure the Raft based metadata server
is to configure metadata-server.type = "replicated".
Rename MetadataStoreClientOptions into MetadataClientOptions
This commit also moves some of the metadata client specific options back
under the [metadata-client] key. It tries to achieve backwards compatibility
with the previous configuration names. The metadata client used with the local
and the replicated metadata server is now called "native" ("embedded" still works
as an alias).