You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2023/01/10 17:17:52 UTC
[GitHub] [incubator-kvrocks] torwig opened a new pull request, #1222: Kvrocks2Redis: decode slot ID in cluster mode
torwig opened a new pull request, #1222:
URL: https://github.com/apache/incubator-kvrocks/pull/1222
Currently, `Kvrocks2Redis` doesn't decode the slot ID if a cluster mode is enabled.
For example, if we write to `Kvrocks` in a cluster mode the following value:
`set book1 "Anything here"`
And run `Kvrocks2Redis` with `cluster-enable` set to `yes`, we will get in `Redis` the following key:
`\x16\x9bbook1`
where `169b` is a hex representation of slot 5787.
The reason: the `Kvrocks` config key `slot_id_encoded` is not set. Setting only the `cluster_enable` config value is not enough for `Kvrocks2Redis`. In `Kvrocks` we have [this](https://github.com/apache/incubator-kvrocks/blob/unstable/src/config/config.cc#L357).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig merged pull request #1222: Kvrocks2Redis: decode slot ID in cluster mode
Posted by GitBox <gi...@apache.org>.
torwig merged PR #1222:
URL: https://github.com/apache/incubator-kvrocks/pull/1222
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] caipengbo commented on a diff in pull request #1222: Kvrocks2Redis: decode slot ID in cluster mode
Posted by GitBox <gi...@apache.org>.
caipengbo commented on code in PR #1222:
URL: https://github.com/apache/incubator-kvrocks/pull/1222#discussion_r1066645837
##########
utils/kvrocks2redis/sync.cc:
##########
@@ -180,8 +180,12 @@ Status Sync::incrementBatchLoop() {
if (bulk_data_str != "ping") {
auto bat = rocksdb::WriteBatch(bulk_data_str);
int count = static_cast<int>(bat.Count());
- parser_->ParseWriteBatch(bulk_data_str);
- auto s = updateNextSeq(next_seq_ + count);
+ auto s = parser_->ParseWriteBatch(bulk_data_str);
+ if (!s.IsOK()) {
+ return s.Prefixed(fmt::format("failed to parse write batch '{}'", Util::StringToHex(bulk_data_str)));
Review Comment:
Get it, I did encounter this error in the replication, because there is a background error, replica can not be written in. I found that the string is very long, and it is not useful for troubleshoting problem, maybe you said, is used for debugging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on pull request #1222: Kvrocks2Redis: decode slot ID in cluster mode
Posted by GitBox <gi...@apache.org>.
torwig commented on PR #1222:
URL: https://github.com/apache/incubator-kvrocks/pull/1222#issuecomment-1378971438
Merging...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1222: Kvrocks2Redis: decode slot ID in cluster mode
Posted by GitBox <gi...@apache.org>.
torwig commented on code in PR #1222:
URL: https://github.com/apache/incubator-kvrocks/pull/1222#discussion_r1066643426
##########
utils/kvrocks2redis/sync.cc:
##########
@@ -180,8 +180,12 @@ Status Sync::incrementBatchLoop() {
if (bulk_data_str != "ping") {
auto bat = rocksdb::WriteBatch(bulk_data_str);
int count = static_cast<int>(bat.Count());
- parser_->ParseWriteBatch(bulk_data_str);
- auto s = updateNextSeq(next_seq_ + count);
+ auto s = parser_->ParseWriteBatch(bulk_data_str);
+ if (!s.IsOK()) {
+ return s.Prefixed(fmt::format("failed to parse write batch '{}'", Util::StringToHex(bulk_data_str)));
Review Comment:
@caipengbo I've just recalled that we use the same approach in replication [(here)](https://github.com/apache/incubator-kvrocks/blob/unstable/src/cluster/replication.cc#L552) and thought that perhaps it was intended to help with debugging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] caipengbo commented on a diff in pull request #1222: Kvrocks2Redis: decode slot ID in cluster mode
Posted by GitBox <gi...@apache.org>.
caipengbo commented on code in PR #1222:
URL: https://github.com/apache/incubator-kvrocks/pull/1222#discussion_r1066536092
##########
utils/kvrocks2redis/sync.cc:
##########
@@ -180,8 +180,12 @@ Status Sync::incrementBatchLoop() {
if (bulk_data_str != "ping") {
auto bat = rocksdb::WriteBatch(bulk_data_str);
int count = static_cast<int>(bat.Count());
- parser_->ParseWriteBatch(bulk_data_str);
- auto s = updateNextSeq(next_seq_ + count);
+ auto s = parser_->ParseWriteBatch(bulk_data_str);
+ if (!s.IsOK()) {
+ return s.Prefixed(fmt::format("failed to parse write batch '{}'", Util::StringToHex(bulk_data_str)));
Review Comment:
Does it help to print out the write batch? This is a long string, and I personally don't think it's very helpful for troubleshooting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1222: Kvrocks2Redis: decode slot ID in cluster mode
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #1222:
URL: https://github.com/apache/incubator-kvrocks/pull/1222#discussion_r1066653940
##########
utils/kvrocks2redis/sync.cc:
##########
@@ -180,8 +180,12 @@ Status Sync::incrementBatchLoop() {
if (bulk_data_str != "ping") {
auto bat = rocksdb::WriteBatch(bulk_data_str);
int count = static_cast<int>(bat.Count());
- parser_->ParseWriteBatch(bulk_data_str);
- auto s = updateNextSeq(next_seq_ + count);
+ auto s = parser_->ParseWriteBatch(bulk_data_str);
+ if (!s.IsOK()) {
+ return s.Prefixed(fmt::format("failed to parse write batch '{}'", Util::StringToHex(bulk_data_str)));
Review Comment:
Yes, it can help us inspect the batch if it goes wrong, so it's fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org