fix: PkPatternMatchDel inconsistent between rediscache and db #2839

chejinge · 2024-08-01T08:50:47Z

No description provided.

coderabbitai · 2024-08-01T08:50:53Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

src/storage/src/redis_hashes.cc

wangshao1 · 2024-08-05T10:35:29Z

src/pika_admin.cc

+  if(s_.ok()) {
+    std::vector<std::string> v;
+    for (auto key : remove_keys_) {
+      v.emplace_back(PCacheKeyPrefixK + key);


pkpatternmatchdel 3.5版本里应该是支持指定数据类型的，所以这里直接把所有类型的匹配key删除是否合理。
还有下边记录binlog那里。

pkpatternmatchdel 3.5版本里应该是支持指定数据类型的，所以这里直接把所有类型的匹配key删除是否合理。还有下边记录binlog那里。

done

wangshao1 · 2024-08-06T02:13:33Z

src/pika_admin.cc

@@ -3098,15 +3098,68 @@ void PKPatternMatchDelCmd::DoInitial() {
    res_.SetRes(CmdRes::kInvalidDbType, kCmdNamePKPatternMatchDel);
    return;
  }
+  max_count_ = storage::BATCH_DELETE_LIMIT;
+  if (argv_.size() > 2) {
+    if (pstd::string2int(argv_[2].data(), argv_[2].size(), &max_count_) == 0 ||  max_count_ < 1 || max_count_ > storage::BATCH_DELETE_LIMIT) {


argv_[2]是类型，这里应该是argv[3]，上边的if条件也得是>3

argv_[2]是类型，这里应该是argv[3]，上边的if条件也得是>3

done

wangshao1 · 2024-08-06T02:19:02Z

src/storage/src/redis_hashes.cc

    key = iter->key().ToString();
    meta_value = iter->value().ToString();
    ParsedHashesMetaValue parsed_hashes_meta_value(&meta_value);
    if (!parsed_hashes_meta_value.IsStale() && (parsed_hashes_meta_value.count() != 0) &&
        (StringMatch(pattern.data(), pattern.size(), key.data(), key.size(), 0) != 0)) {
      parsed_hashes_meta_value.InitialMetaValue();
      batch.Put(handles_[0], key, meta_value);
+      remove_keys->push_back(key.data());


key已经是string类型了吧？直接push_back(key)就可以了。

key已经是string类型了吧？直接push_back(key)就可以了。

done

wangshao1 · 2024-08-06T02:29:00Z

src/storage/src/redis_lists.cc

  Status s;
  rocksdb::WriteBatch batch;
  rocksdb::Iterator* iter = db_->NewIterator(iterator_options, handles_[0]);
  DEFER {
    delete iter;
  };
  iter->SeekToFirst();
-  while (iter->Valid()) {
+  while (iter->Valid() && static_cast<int64_t>(batch.Count()) < max_count) {


182行的逻辑，里边会执行batch.clear，所以用batch.Count()去比不准确。要不你就跟4.0里的一样，把182 if里的删掉，统一在外边调用一次batch.put。

182行的逻辑，里边会执行batch.clear，所以用batch.Count()去比不准确。要不你就跟4.0里的一样，把182 if里的删掉，统一在外边调用一次batch.put。

done

wangshao1 · 2024-08-06T02:33:09Z

src/storage/src/storage.cc

  Status s;
+  int64_t tmp_ret = 0;


3.5里不需要这个tmp_ret，因为他只会删一个数据类型的，这个是个switch，不是for循环，直接把max_count传进去就行。

wangshao1 · 2024-08-06T02:33:36Z

src/storage/tests/keys_test.cc

@@ -2461,6 +2460,7 @@ for (const auto& kv : kvs) {

 //   //=============================== List ===============================

+


多出来的空行需要删掉。

多出来的空行需要删掉。

done

luky116 · 2024-08-07T02:27:24Z

src/storage/src/redis_strings.cc

      batch.Clear();
+    } else {
+      remove_keys->erase(remove_keys->end() - batch.Count(), remove_keys->end());
    }
  }
  delete iter;


移除多余的 delete iter;

luky116 · 2024-08-07T02:38:04Z

src/storage/src/redis_sets.cc

      batch.Clear();
+    } else {
+      remove_keys->erase(remove_keys->end() - batch.Count(), remove_keys->end());


待确认语法语义是否OK

待确认语法语义是否OK

这里因为只有一种数据类型 batch不可能出现一部分失败的情况所以暂时使用clear

luky116 · 2024-08-07T02:39:15Z

src/storage/src/storage.cc

      break;
    case DataType::kSets:
-      s = sets_db_->PKPatternMatchDel(pattern, ret);
+      s = sets_db_->PKPatternMatchDelWithRemoveKeys(DataType::kSets, pattern, ret,remove_keys, max_count - *ret);


加空格

done

AlexStocks · 2024-08-07T04:14:23Z

src/storage/src/redis_strings.cc

      batch.Clear();
+    } else {
+      remove_keys->clear();


remove_keys.erase(remove_keys.end() - batchNum, remove_keys.end());

remove_keys.erase(remove_keys.end() - batchNum, remove_keys.end());

done

AlexStocks · 2024-08-07T04:15:14Z

src/storage/src/redis_strings.cc

-        delete iter;
-        return s;
-      }
+      remove_keys->push_back(key);
    }
    iter->Next();
  }


auto batchNum = batch.Count();

auto batchNum = batch.Count();

done

…omFoundation#2839) * fix: PkPatternMatchDel inconsistent between rediscache and db --------- Co-authored-by: haiyang426 <[email protected]> Co-authored-by: chejinge <[email protected]>

fix: PkPatternMatchDel inconsistent between rediscache and db

fa414f1

github-actions bot added the ☢️ Bug Something isn't working label Aug 1, 2024

AlexStocks added the 3.5.5 label Aug 5, 2024

chejinge force-pushed the pkpattern branch from 397891c to a7a9fd6 Compare August 5, 2024 08:09

wangshao1 reviewed Aug 5, 2024

View reviewed changes

chejinge force-pushed the pkpattern branch 3 times, most recently from 76a923f to c730b71 Compare August 5, 2024 11:51

fix

407ac39

chejinge force-pushed the pkpattern branch from c730b71 to 407ac39 Compare August 5, 2024 11:58

wangshao1 reviewed Aug 6, 2024

View reviewed changes

chejinge force-pushed the pkpattern branch from 11cedc1 to ffeb9a4 Compare August 6, 2024 03:53

luky116 reviewed Aug 7, 2024

View reviewed changes

chejinge force-pushed the pkpattern branch 2 times, most recently from 8f09837 to 5f22ac2 Compare August 7, 2024 03:00

wangshao1 approved these changes Aug 7, 2024

View reviewed changes

AlexStocks reviewed Aug 7, 2024

View reviewed changes

fix review change

9fa82ee

chejinge force-pushed the pkpattern branch from 5f22ac2 to 9fa82ee Compare August 7, 2024 04:31

Mixficsol approved these changes Aug 7, 2024

View reviewed changes

chejinge merged commit 4a9872c into OpenAtomFoundation:3.5 Aug 7, 2024
11 of 12 checks passed

chejinge deleted the pkpattern branch November 4, 2024 07:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: PkPatternMatchDel inconsistent between rediscache and db #2839

fix: PkPatternMatchDel inconsistent between rediscache and db #2839

chejinge commented Aug 1, 2024

coderabbitai bot commented Aug 1, 2024 •

edited

Loading

Review skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

wangshao1 Aug 5, 2024

chejinge Aug 5, 2024

wangshao1 Aug 6, 2024

chejinge Aug 6, 2024

wangshao1 Aug 6, 2024

chejinge Aug 6, 2024

wangshao1 Aug 6, 2024

chejinge Aug 6, 2024

wangshao1 Aug 6, 2024

wangshao1 Aug 6, 2024

chejinge Aug 6, 2024

luky116 Aug 7, 2024

luky116 Aug 7, 2024

chejinge Aug 7, 2024

luky116 Aug 7, 2024

chejinge Aug 7, 2024

AlexStocks Aug 7, 2024 •

edited

Loading

chejinge Aug 7, 2024

AlexStocks Aug 7, 2024

chejinge Aug 7, 2024

		@@ -2461,6 +2460,7 @@ for (const auto& kv : kvs) {

		// //=============================== List ===============================

fix: PkPatternMatchDel inconsistent between rediscache and db #2839

fix: PkPatternMatchDel inconsistent between rediscache and db #2839

Conversation

chejinge commented Aug 1, 2024

coderabbitai bot commented Aug 1, 2024 • edited Loading

Review skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexStocks Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderabbitai bot commented Aug 1, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

AlexStocks Aug 7, 2024 •

edited

Loading