support slot batch for CLUSTERX SETSLOT #1414

infdahai · 2023-05-01T18:31:04Z

closes #1412. refer to #529

add stdint.h for uint_t type
ref(https://godbolt.org/z/5q6zj5W9s)
add cluster_setslot_batch feature.
tests for parsedSlotRange(cpp unit and go test)
move parse func to commandX

src/cluster/cluster.cc

src/commands/cmd_cluster.cc

PragmaTwice

Thanks for your contribution!

You can run ./x.py format to format the code and let the CI proceed.

src/common/encoding.h

CMakeLists.txt

x.py

src/cluster/redis_slot.h

src/cluster/cluster.cc

PragmaTwice · 2023-05-02T07:42:42Z

If I exec ./x.py format, I get a irrelevant result.(Not important though, just ignored).

Could you provide the version of clang-format you are using?

Anyway, I think you can firstly manually ignore this unrelated change as a workaround.

PragmaTwice · 2023-05-02T07:43:40Z

Sorry for the wrong close-button click. 🤣

PragmaTwice · 2023-05-02T08:05:23Z

CI seems still blocked in format. Not sure your clang-format version, but you can try clang-format 12, which is used in CI.

https://github.com/llvm/llvm-project/releases/tag/llvmorg-12.0.1

And I can confirm that clang-format 14 on my local has also no issue.

/home/runner/work/incubator-kvrocks/incubator-kvrocks/src/cluster/cluster.cc:474:39: error: code should be clang-formatted [-Wclang-format-violations]
  for (const auto &id : n->replicas) {      // replicas
                                      ^

infdahai · 2023-05-02T08:16:00Z

CI seems still blocked in format. Not sure your clang-format version, but you can try clang-format 12, which is used in CI.

https://github.com/llvm/llvm-project/releases/tag/llvmorg-12.0.1

And I can confirm that clang-format 14 on my local has also no issue.
/home/runner/work/incubator-kvrocks/incubator-kvrocks/src/cluster/cluster.cc:474:39: error: code should be clang-formatted [-Wclang-format-violations]
  for (const auto &id : n->replicas) {      // replicas
                                      ^

Sorry for this problem. My llvm toolchain version is not unified and the clang-format version is 16.

You're too kind. Ignore this problem.

src/cluster/cluster.cc

PragmaTwice

LGTM generally, it would be better if you add several golang tests to tests/gocase/integration/cluster/cluster_test.go .

src/commands/cmd_cluster.cc

src/cluster/cluster.cc

src/cluster/cluster_defs.h

infdahai · 2023-05-05T06:24:26Z

src/cluster/cluster.cc

+      if (old_node == myself_ && old_node != to_assign_node) {
+        // If slot is migrated from this node
+        if (migrated_slots_.count(slot) > 0) {
+          svr_->slot_migrator->ClearKeysOfSlot(kDefaultNamespace, slot);


One ClearKeyOfSlot call one writebatch of rocksdb.
So this process exists consistency issues if slot in the middle is down.
I don't know what to do with it yet

There is indeed this problem and we can persist it to Propagate CF.

After we merge this PR, you can ask an issue and I will solve this problem, which I have solved in production environment.

src/cluster/cluster.cc

git-hulk · 2023-05-05T06:54:02Z

LGTM, I think we can remove TODO comments, then submit an issue if you feel it's necessary.

infdahai · 2023-05-05T07:19:09Z

LGTM, I think we can remove TODO comments, then submit an issue if you feel it's necessary.

OK

1. ref(https://godbolt.org/z/5q6zj5W9s) Signed-off-by: clundro <859287553@qq.com>

1. add cluster_slot_batch ref to apache#529 2. add tests for parsedSlotRange 3. move parse func to commandX Signed-off-by: clundro <859287553@qq.com>

caipengbo

LGTM

torwig

LGTM

git-hulk · 2023-05-14T11:53:11Z

Thanks all, merging...

mapleFU reviewed May 1, 2023

View reviewed changes

src/cluster/cluster.cc Outdated Show resolved Hide resolved

src/cluster/cluster.cc Outdated Show resolved Hide resolved

src/cluster/cluster.cc Outdated Show resolved Hide resolved

torwig reviewed May 1, 2023

View reviewed changes

infdahai changed the title ~~support for cluster slot batch~~ support for slot batch for CLUSTERX SETSLOT May 2, 2023

infdahai changed the title ~~support for slot batch for CLUSTERX SETSLOT~~ support slot batch for CLUSTERX SETSLOT May 2, 2023

infdahai force-pushed the cluster_slot_batch branch from ac3e381 to 2a340e6 Compare May 2, 2023 01:39

PragmaTwice reviewed May 2, 2023

View reviewed changes

This comment was marked as off-topic.

Sign in to view

PragmaTwice closed this May 2, 2023

PragmaTwice reopened this May 2, 2023

This comment was marked as off-topic.

Sign in to view

infdahai force-pushed the cluster_slot_batch branch from 5d48663 to 5d4cf69 Compare May 2, 2023 08:11

git-hulk reviewed May 3, 2023

View reviewed changes

src/cluster/cluster.cc Outdated Show resolved Hide resolved

src/cluster/cluster.cc Outdated Show resolved Hide resolved

src/cluster/cluster.cc Outdated Show resolved Hide resolved

infdahai force-pushed the cluster_slot_batch branch 3 times, most recently from d245e9c to fa57cb7 Compare May 3, 2023 10:42

PragmaTwice reviewed May 3, 2023

View reviewed changes

caipengbo reviewed May 3, 2023

View reviewed changes

src/commands/cmd_cluster.cc Outdated Show resolved Hide resolved

src/cluster/cluster.cc Outdated Show resolved Hide resolved

infdahai force-pushed the cluster_slot_batch branch 2 times, most recently from 6f9eeab to ea4d676 Compare May 4, 2023 18:56

caipengbo previously approved these changes May 5, 2023

View reviewed changes

src/cluster/cluster.cc Outdated Show resolved Hide resolved

infdahai dismissed caipengbo’s stale review via 032cb82 May 5, 2023 06:11

infdahai force-pushed the cluster_slot_batch branch from ea4d676 to 032cb82 Compare May 5, 2023 06:11

PragmaTwice reviewed May 5, 2023

View reviewed changes

src/cluster/cluster_defs.h Outdated Show resolved Hide resolved

infdahai commented May 5, 2023

View reviewed changes

infdahai force-pushed the cluster_slot_batch branch 2 times, most recently from 333f37b to 870afd2 Compare May 5, 2023 06:37

git-hulk reviewed May 5, 2023

View reviewed changes

src/cluster/cluster.cc Outdated Show resolved Hide resolved

infdahai force-pushed the cluster_slot_batch branch 2 times, most recently from 8aee7e9 to e8a5bd4 Compare May 14, 2023 05:57

infdahai added 2 commits May 14, 2023 14:00

add stdint.h for uint_t type

47375d1

1. ref(https://godbolt.org/z/5q6zj5W9s) Signed-off-by: clundro <859287553@qq.com>

support cluster_set_slots batch

0ad6d85

1. add cluster_slot_batch ref to apache#529 2. add tests for parsedSlotRange 3. move parse func to commandX Signed-off-by: clundro <859287553@qq.com>

infdahai force-pushed the cluster_slot_batch branch from e8a5bd4 to 0ad6d85 Compare May 14, 2023 06:00

infdahai requested a review from git-hulk May 14, 2023 07:54

caipengbo approved these changes May 14, 2023

View reviewed changes

torwig approved these changes May 14, 2023

View reviewed changes

git-hulk approved these changes May 14, 2023

View reviewed changes

git-hulk merged commit d2d10f7 into apache:unstable May 14, 2023

infdahai deleted the cluster_slot_batch branch May 14, 2023 11:54

infdahai mentioned this pull request May 14, 2023

support transcation processing for cluster setslot #1439

Open

2 tasks

PokIsemaine mentioned this pull request Jun 21, 2024

Support passing a slot range in CLUSTERX MIGRATE #2355

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support slot batch for CLUSTERX SETSLOT #1414

support slot batch for CLUSTERX SETSLOT #1414

infdahai commented May 1, 2023 •

edited

Loading

PragmaTwice left a comment

This comment was marked as off-topic.

PragmaTwice commented May 2, 2023 •

edited

Loading

PragmaTwice commented May 2, 2023

This comment was marked as off-topic.

PragmaTwice commented May 2, 2023 •

edited

Loading

infdahai commented May 2, 2023 •

edited

Loading

PragmaTwice left a comment

infdahai May 5, 2023

caipengbo May 5, 2023 •

edited

Loading

caipengbo May 5, 2023

git-hulk commented May 5, 2023

infdahai commented May 5, 2023

caipengbo left a comment

torwig left a comment

git-hulk commented May 14, 2023

support slot batch for CLUSTERX SETSLOT #1414

support slot batch for CLUSTERX SETSLOT #1414

Conversation

infdahai commented May 1, 2023 • edited Loading

PragmaTwice left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

PragmaTwice commented May 2, 2023 • edited Loading

PragmaTwice commented May 2, 2023

This comment was marked as off-topic.

PragmaTwice commented May 2, 2023 • edited Loading

infdahai commented May 2, 2023 • edited Loading

PragmaTwice left a comment

Choose a reason for hiding this comment

infdahai May 5, 2023

Choose a reason for hiding this comment

caipengbo May 5, 2023 • edited Loading

Choose a reason for hiding this comment

caipengbo May 5, 2023

Choose a reason for hiding this comment

git-hulk commented May 5, 2023

infdahai commented May 5, 2023

caipengbo left a comment

Choose a reason for hiding this comment

torwig left a comment

Choose a reason for hiding this comment

git-hulk commented May 14, 2023

infdahai commented May 1, 2023 •

edited

Loading

PragmaTwice commented May 2, 2023 •

edited

Loading

PragmaTwice commented May 2, 2023 •

edited

Loading

infdahai commented May 2, 2023 •

edited

Loading

caipengbo May 5, 2023 •

edited

Loading