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

Persist the cluster-enabled status in RocksDB #1410

Open
1 of 2 tasks
git-hulk opened this issue Apr 29, 2023 · 9 comments · May be fixed by #2020
Open
1 of 2 tasks

Persist the cluster-enabled status in RocksDB #1410

git-hulk opened this issue Apr 29, 2023 · 9 comments · May be fixed by #2020
Assignees
Labels
enhancement type enhancement

Comments

@git-hulk
Copy link
Member

Search before asking

  • I had searched in the issues and found no similar issues.

Motivation

Currently, we encode the slot id as the prefix of the key if the cluster mode was enabled according to Kvrocks's configuration. But it would go wrong if we start the Kvrocks with cluster-enabled yes, but change it to cluster-enabled no after restarting.

For the key encoding can see: https://kvrocks.apache.org/community/data-structure-on-rocksdb

Solution

Persist the cluster-enabled configuration in the RocksDB(can use propagate column family) when starting the server, then check if the Kvrocks's cluster-enabled configuration matched the database status.

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@git-hulk git-hulk added the enhancement type enhancement label Apr 29, 2023
@infdahai
Copy link
Contributor

Let's try this.

@git-hulk
Copy link
Member Author

@infdahai Assigned, let's go ahead.

@infdahai
Copy link
Contributor

infdahai commented May 1, 2023

@git-hulk I find many go tests use the config map including "cluster-enabled": "yes" and the StartServerWithCLIOptions function writes the option to the kvrocks.conf file.

func TestClusterDumpAndLoadClusterNodesInfo(t *testing.T) {
	srv1 := util.StartServer(t, map[string]string{
		"bind":            "0.0.0.0",
		"cluster-enabled": "yes",
	})

in StartServerWithCLIOptions func(),
https://github.com/apache/incubator-kvrocks/blob/30e9fd788a4acb585566a008f4d5fff65ee06ce3/tests/gocase/util/server.go#L197-L207

I haven't seen a commad that changes cluster mode, so it means we should define the option in the initial config file if we want to use cluster mode. Restart() will read the kvrocks.conf and set the origin value.
https://github.com/apache/incubator-kvrocks/blob/30e9fd788a4acb585566a008f4d5fff65ee06ce3/tests/gocase/util/server.go#L116-L128

change it to cluster-enabled no` after restarting.

so I don't know how this happens, could you explain it?

@git-hulk
Copy link
Member Author

Hi @infdahai

Sorry for missing this comment.

I haven't seen a commad that changes cluster mode, so it means we should define the option in the initial config file if we want to use cluster mode. Restart() will read the kvrocks.conf and set the origin value.

Yeah, that's right that we have no command to set the cluster mode, it must be specified in the configuration file. But the current restart function cannot change the kvrocks.conf, so you need to support overriding the configuration in the restart function first if wanna test this scenario.

@git-hulk
Copy link
Member Author

@inf Are you still working on this?

@infdahai
Copy link
Contributor

@git-hulk I think I know what to do. A new pull request is on the way.

@git-hulk
Copy link
Member Author

Cool, thanks. No hurry, just for asking if you're working on this.

@chrisxu333
Copy link
Contributor

@infdahai are you still working on this? If not I'd like to take over. @git-hulk

@git-hulk git-hulk assigned chrisxu333 and unassigned infdahai Jan 14, 2024
@git-hulk
Copy link
Member Author

@chrisxu333 Done, thank you!

@chrisxu333 chrisxu333 linked a pull request Jan 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement type enhancement
Projects
None yet
3 participants