-
Notifications
You must be signed in to change notification settings - Fork 753
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
Rewrite the correct databases value when dbnum is clamped to 1 in cluster mode #1856
base: unstable
Are you sure you want to change the base?
Conversation
…ster mode In cb1fff3, we will override the databases config at startup to 1 in cluster mode. When the user than calls CONFIG REWRITE, it will write out the value of 1 to the config file. We should still write out the configured value in this case. This closes valkey-io#1853. Signed-off-by: Binbin <[email protected]>
Signed-off-by: Binbin <[email protected]>
Signed-off-by: Binbin <[email protected]>
@@ -0,0 +1,8 @@ | |||
start_cluster 1 0 {tags {external:skip cluster}} { | |||
test "CONFIG REWRITE databases" { | |||
assert_equal 16 [lindex [r config get databases] 1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noted that with this change, config get will always get the config_dbnum
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #1856 +/- ##
============================================
- Coverage 71.09% 70.98% -0.12%
============================================
Files 123 123
Lines 65671 65672 +1
============================================
- Hits 46687 46614 -73
- Misses 18984 19058 +74
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be treated as a breaking change?
i am guessing @madolson brought it up with the intention of including it in 8.1? OR just a tmp fix that lives in unstable unit we get multdb support in 9.0 |
not sure about it, i'd probably prefer it to be a bugfix. |
I think we can release it in 9.0 together with cluster-databases. They are related. No reason to confuse users about this in 8.1 release notes. |
If I understood correctly, we decided in the meeting two days ago that we will do this change. We can already mark it as major-decision-approved? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool.
In cb1fff3, we will override the databases
config at startup to 1 in cluster mode. When the user than calls CONFIG REWRITE,
it will write out the value of 1 to the config file. We should still write
out the configured value in this case.
This closes #1853.