Make ScyllaDB replication factor configurable #3788
Merged
Conversation
This file contains hidden or 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
This was referenced Apr 16, 2025
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
6fd307a to
5e7a71d
Compare
ma2bd
reviewed
Apr 17, 2025
linera-service/src/linera/main.rs
Outdated
| debug!("Running command using storage configuration: {storage_config}"); | ||
| let store_config = storage_config | ||
| .add_common_config(self.common_config()) | ||
| .add_common_config(self.common_config(), self.storage_replication_factor) |
Contributor
There was a problem hiding this comment.
Isn't it what the common config was about though?
Contributor
Author
There was a problem hiding this comment.
Ok, I can add it to the common config
Contributor
MathieuDutSik
left a comment
MathieuDutSik
left a comment
There was a problem hiding this comment.
First comment is that the addition of a storage-replication-factor is generally a good idea. I think that in operations, the ScyllaDb instance is not spun out in that way. But, it is a nice thing to have it available for tests.
Where I think this is more problematic is where should we put this setting.
As is apparent, this is not an internal setting of ScyllaDB. The ScyllaDB instance is already running, either in Docker or some other system. Therefore, the replication factor should be next to the uri and should be obtained when parsing the --storage entry.
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
5e7a71d to
8691581
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
8691581 to
e6786ca
Compare
ndr-ds
changed the base branch from
graphite-base/3788
to
05-02-add_local_ssd_support
This was referenced May 2, 2025
Merged
Merged
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
e6786ca to
11e93bc
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
1082be6 to
c0bae58
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
3 times, most recently
from
6b804f1 to
4930e8e
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
c0bae58 to
fa8dbb4
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
4930e8e to
22d7171
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
fa8dbb4 to
dac015a
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
22d7171 to
32eb499
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
dac015a to
b95af69
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
32eb499 to
05a929a
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
b95af69 to
f5e964e
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
05a929a to
0d5275a
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
f5e964e to
a078dbc
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
0d5275a to
46bf043
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
a078dbc to
48fbe06
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
46bf043 to
5f0e62a
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
48fbe06 to
c9e259d
Compare
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
5f0e62a to
1971bcf
Compare
ma2bd
reviewed
May 7, 2025
| impl TestKeyValueStore for JournalingKeyValueStore<ScyllaDbStoreInternal> { | ||
| async fn new_test_config() -> Result<ScyllaDbStoreInternalConfig, ScyllaDbStoreInternalError> { | ||
| let uri = "localhost:9042".to_string(); | ||
| let common_config = CommonStoreInternalConfig { |
Contributor
There was a problem hiding this comment.
I'm trying to remove this common config because most options there only work for a subset of the databases
afck
approved these changes
May 8, 2025
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
1971bcf to
2a07fcc
Compare
ndr-ds
force-pushed
the
05-02-add_local_ssd_support
branch
from
c9e259d to
1cf0afb
Compare
This was referenced May 9, 2025
Contributor
Author
ndr-ds
changed the base branch from
05-02-add_local_ssd_support
to
graphite-base/3788
ndr-ds
force-pushed
the
04-14-make_scylladb_replication_factor_configurable
branch
from
2a07fcc to
4f383ee
Compare
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.
Motivation
We'll use replication factor of 1 in CI and locally, but probably will use 3 in deployed networks on GCP
Proposal
Add an option to support that
Test Plan
CI + deployed network locally
Release Plan