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.

Sign up for GitHub

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

Jump to bottom

persist cluster-enabled status in RocksDB #2020

Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from
Open

persist cluster-enabled status in RocksDB #2020

wants to merge 3 commits into from

Conversation

chrisxu333
Copy link
Contributor

To close #1410 .

@chrisxu333
Copy link
Contributor Author

@git-hulk Please help review :)

caipengbo
caipengbo reviewed Jan 15, 2024
View reviewed changes
src/server/server.cc Outdated Show resolved Hide resolved
src/server/server.cc Outdated Show resolved Hide resolved
src/server/server.cc Outdated Show resolved Hide resolved
PragmaTwice
PragmaTwice reviewed Jan 15, 2024
View reviewed changes
src/storage/storage.h Outdated Show resolved Hide resolved
@torwig
Copy link
Contributor

torwig commented Jan 15, 2024

@chrisxu333 Please, run ./x.py format to format the codebase and CI will be happy :)

@chrisxu333 chrisxu333 force-pushed the persist_cluster-enabled_option branch from 244f927 to 74f245b Compare January 15, 2024 05:47
git-hulk
git-hulk reviewed Jan 15, 2024
View reviewed changes
src/server/server.cc Outdated Show resolved Hide resolved
src/server/server.cc Outdated Show resolved Hide resolved
src/server/server.cc Outdated Show resolved Hide resolved
src/server/server.cc Outdated Show resolved Hide resolved
@chrisxu333 chrisxu333 force-pushed the persist_cluster-enabled_option branch from 74f245b to f3b8266 Compare January 15, 2024 08:46
PragmaTwice
PragmaTwice reviewed Jan 15, 2024
View reviewed changes
src/server/server.cc Outdated Show resolved Hide resolved
PragmaTwice
PragmaTwice reviewed Jan 15, 2024
View reviewed changes
src/server/server.cc Outdated Show resolved Hide resolved
PragmaTwice
PragmaTwice reviewed Jan 15, 2024
View reviewed changes
src/server/server.cc Outdated Show resolved Hide resolved
PragmaTwice
PragmaTwice reviewed Jan 15, 2024
View reviewed changes
src/server/server.cc Outdated Show resolved Hide resolved
@chrisxu333 chrisxu333 force-pushed the persist_cluster-enabled_option branch from f3151b9 to 034759b Compare January 15, 2024 14:46
PragmaTwice
PragmaTwice reviewed Jan 15, 2024
View reviewed changes
src/server/server.cc Outdated Show resolved Hide resolved
@chrisxu333 chrisxu333 force-pushed the persist_cluster-enabled_option branch from 034759b to f4d36b5 Compare January 15, 2024 15:00
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.7% Duplication on New Code

See analysis details on SonarCloud

@chrisxu333
Copy link
Contributor Author

chrisxu333 commented Jan 15, 2024 •
edited
Loading

Hi @git-hulk could you also help diagnose why the newly introduced testcase is failed? It was successful when I did locally.

@git-hulk
Copy link
Member

git-hulk commented Jan 16, 2024 •
edited
Loading

Hi @git-hulk could you also help diagnose why the newly introduced testcase is failed? It was successful when I did locally.

perhaps you can try to debug the test case yourself, and I will also take a look while I'm free.

@git-hulk git-hulk mentioned this pull request Jan 23, 2024
Closed
PragmaTwice
PragmaTwice reviewed May 5, 2024
View reviewed changes
src/server/server.cc
@@ -152,6 +152,11 @@ Status Server::Start() {
}
}

s = checkClusterMode();
Copy link
Member

@PragmaTwice PragmaTwice May 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Seems this should be before the namespace_.LoadAndRewrite(). To avoid some weird error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PragmaTwice PragmaTwice PragmaTwice left review comments

@caipengbo caipengbo caipengbo left review comments

@git-hulk git-hulk git-hulk left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
Kvrocks Primary Bug Tracker
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Persist the cluster-enabled status in RocksDB
5 participants
@chrisxu333 @torwig @git-hulk @caipengbo @PragmaTwice