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

fix: duplicate meta key read #2744

Merged
merged 1 commit into from
Jun 21, 2024
Merged

fix: duplicate meta key read #2744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 36 additions & 36 deletions src/storage/src/redis.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,36 +117,36 @@ class Redis {
Status ScanStreamsKeyNum(KeyInfo* key_info);

// Keys Commands
virtual Status StringsExpire(const Slice& key, int64_t ttl);
virtual Status HashesExpire(const Slice& key, int64_t ttl);
virtual Status ListsExpire(const Slice& key, int64_t ttl);
virtual Status ZsetsExpire(const Slice& key, int64_t ttl);
virtual Status SetsExpire(const Slice& key, int64_t ttl);

virtual Status StringsDel(const Slice& key);
virtual Status HashesDel(const Slice& key);
virtual Status ListsDel(const Slice& key);
virtual Status ZsetsDel(const Slice& key);
virtual Status SetsDel(const Slice& key);
virtual Status StreamsDel(const Slice& key);

virtual Status StringsExpireat(const Slice& key, int64_t timestamp);
virtual Status HashesExpireat(const Slice& key, int64_t timestamp);
virtual Status ListsExpireat(const Slice& key, int64_t timestamp);
virtual Status SetsExpireat(const Slice& key, int64_t timestamp);
virtual Status ZsetsExpireat(const Slice& key, int64_t timestamp);

virtual Status StringsPersist(const Slice& key);
virtual Status HashesPersist(const Slice& key);
virtual Status ListsPersist(const Slice& key);
virtual Status ZsetsPersist(const Slice& key);
virtual Status SetsPersist(const Slice& key);

virtual Status StringsTTL(const Slice& key, int64_t* timestamp);
virtual Status HashesTTL(const Slice& key, int64_t* timestamp);
virtual Status ListsTTL(const Slice& key, int64_t* timestamp);
virtual Status ZsetsTTL(const Slice& key, int64_t* timestamp);
virtual Status SetsTTL(const Slice& key, int64_t* timestamp);
virtual Status StringsExpire(const Slice& key, int64_t ttl, std::string&& prefetch_meta = {});
virtual Status HashesExpire(const Slice& key, int64_t ttl, std::string&& prefetch_meta = {});
virtual Status ListsExpire(const Slice& key, int64_t ttl, std::string&& prefetch_meta = {});
virtual Status ZsetsExpire(const Slice& key, int64_t ttl, std::string&& prefetch_meta = {});
virtual Status SetsExpire(const Slice& key, int64_t ttl, std::string&& prefetch_meta = {});

virtual Status StringsDel(const Slice& key, std::string&& prefetch_meta = {});
virtual Status HashesDel(const Slice& key, std::string&& prefetch_meta = {});
virtual Status ListsDel(const Slice& key, std::string&& prefetch_meta = {});
virtual Status ZsetsDel(const Slice& key, std::string&& prefetch_meta = {});
virtual Status SetsDel(const Slice& key, std::string&& prefetch_meta = {});
virtual Status StreamsDel(const Slice& key, std::string&& prefetch_meta = {});

virtual Status StringsExpireat(const Slice& key, int64_t timestamp, std::string&& prefetch_meta = {});
virtual Status HashesExpireat(const Slice& key, int64_t timestamp, std::string&& prefetch_meta = {});
virtual Status ListsExpireat(const Slice& key, int64_t timestamp, std::string&& prefetch_meta = {});
virtual Status SetsExpireat(const Slice& key, int64_t timestamp, std::string&& prefetch_meta = {});
virtual Status ZsetsExpireat(const Slice& key, int64_t timestamp, std::string&& prefetch_meta = {});

virtual Status StringsPersist(const Slice& key, std::string&& prefetch_meta = {});
virtual Status HashesPersist(const Slice& key, std::string&& prefetch_meta = {});
virtual Status ListsPersist(const Slice& key, std::string&& prefetch_meta = {});
virtual Status ZsetsPersist(const Slice& key, std::string&& prefetch_meta = {});
virtual Status SetsPersist(const Slice& key, std::string&& prefetch_meta = {});

virtual Status StringsTTL(const Slice& key, int64_t* timestamp, std::string&& prefetch_meta = {});
virtual Status HashesTTL(const Slice& key, int64_t* timestamp, std::string&& prefetch_meta = {});
virtual Status ListsTTL(const Slice& key, int64_t* timestamp, std::string&& prefetch_meta = {});
virtual Status ZsetsTTL(const Slice& key, int64_t* timestamp, std::string&& prefetch_meta = {});
virtual Status SetsTTL(const Slice& key, int64_t* timestamp, std::string&& prefetch_meta = {});

// Strings Commands
Status Append(const Slice& key, const Slice& value, int32_t* ret);
Expand Down Expand Up @@ -200,7 +200,7 @@ class Redis {
Status HIncrby(const Slice& key, const Slice& field, int64_t value, int64_t* ret);
Status HIncrbyfloat(const Slice& key, const Slice& field, const Slice& by, std::string* new_value);
Status HKeys(const Slice& key, std::vector<std::string>* fields);
Status HLen(const Slice& key, int32_t* ret);
Status HLen(const Slice& key, int32_t* ret, std::string&& prefetch_meta = {});
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The results indicate that multiple call sites of the HLen method have not been updated to include the new std::string&& prefetch_meta parameter. Specifically, the following files contain calls to HLen that need to be updated:

  • src/storage/tests/hashes_test.cc
  • src/storage/examples/hashes_example.cc
  • src/storage/src/storage.cc
  • src/pika_hash.cc
  • src/pika_cache_load_thread.cc
  • src/pika_cache.cc

These files should be reviewed and modified to ensure that the new prefetch_meta parameter is included in all calls to HLen.

  • src/storage/tests/hashes_test.cc: Multiple instances.
  • src/storage/examples/hashes_example.cc: One instance.
  • src/storage/src/storage.cc: One instance.
  • src/pika_hash.cc: Multiple instances.
  • src/pika_cache_load_thread.cc: One instance.
  • src/pika_cache.cc: Two instances.
Analysis chain

The HLen method now accepts a std::string&& prefetch_meta parameter. It's crucial to ensure that this change does not disrupt the existing functionality and that all calls to HLen across the codebase are updated to include this new parameter.

The rg output indicates that there are several call sites for the HLen method in various files:

  • src/storage/tests/hashes_test.cc
  • src/storage/examples/hashes_example.cc
  • src/storage/src/storage.cc
  • src/pika_hash.cc
  • src/pika_cache_load_thread.cc
  • src/pika_cache.cc

To verify that all these calls have been updated to include the std::string&& prefetch_meta parameter, we need to inspect each call site.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Ensure all calls to `HLen` are updated to the new signature.
rg --type cpp 'HLen'

Length of output: 4008

Script:

#!/bin/bash
# Check the call sites of HLen in src/storage/tests/hashes_test.cc
rg 'HLen' src/storage/tests/hashes_test.cc

# Check the call sites of HLen in src/storage/examples/hashes_example.cc
rg 'HLen' src/storage/examples/hashes_example.cc

# Check the call sites of HLen in src/storage/src/storage.cc
rg 'HLen' src/storage/src/storage.cc

# Check the call sites of HLen in src/pika_hash.cc
rg 'HLen' src/pika_hash.cc

# Check the call sites of HLen in src/pika_cache_load_thread.cc
rg 'HLen' src/pika_cache_load_thread.cc

# Check the call sites of HLen in src/pika_cache.cc
rg 'HLen' src/pika_cache.cc

Length of output: 1737

Status HMGet(const Slice& key, const std::vector<std::string>& fields, std::vector<ValueStatus>* vss);
Status HMSet(const Slice& key, const std::vector<FieldValue>& fvs);
Status HSet(const Slice& key, const Slice& field, const Slice& value, int32_t* res);
Expand Down Expand Up @@ -246,7 +246,7 @@ class Redis {

// Sets Commands
Status SAdd(const Slice& key, const std::vector<std::string>& members, int32_t* ret);
Status SCard(const Slice& key, int32_t* ret);
Status SCard(const Slice& key, int32_t* ret, std::string&& prefetch_meta = {});
Status SDiff(const std::vector<std::string>& keys, std::vector<std::string>* members);
Status SDiffstore(const Slice& destination, const std::vector<std::string>& keys, std::vector<std::string>& value_to_dest, int32_t* ret);
Status SInter(const std::vector<std::string>& keys, std::vector<std::string>* members);
Expand All @@ -269,7 +269,7 @@ class Redis {
Status LIndex(const Slice& key, int64_t index, std::string* element);
Status LInsert(const Slice& key, const BeforeOrAfter& before_or_after, const std::string& pivot,
const std::string& value, int64_t* ret);
Status LLen(const Slice& key, uint64_t* len);
Status LLen(const Slice& key, uint64_t* len, std::string&& prefetch_meta = {});
Status LPop(const Slice& key, int64_t count, std::vector<std::string>* elements);
Status LPush(const Slice& key, const std::vector<std::string>& values, uint64_t* ret);
Status LPushx(const Slice& key, const std::vector<std::string>& values, uint64_t* len);
Expand All @@ -285,7 +285,7 @@ class Redis {

// Zsets Commands
Status ZAdd(const Slice& key, const std::vector<ScoreMember>& score_members, int32_t* ret);
Status ZCard(const Slice& key, int32_t* card);
Status ZCard(const Slice& key, int32_t* card, std::string&& prefetch_meta = {});
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The ZCard method update to accept a std::string&& prefetch_meta parameter has been partially verified. The implementation in src/storage/src/redis_zsets.cc matches the new signature, and there are instances in src/storage/src/redis_strings.cc where the method is called with the new parameter. However, many calls to ZCard across the codebase still use the old signature without the std::string&& prefetch_meta parameter.

  • Locations needing attention:
    • src/storage/tests/zsets_test.cc
    • src/storage/tests/keys_test.cc
    • src/storage/src/storage.cc
    • src/pika_cache_load_thread.cc
    • src/pika_cache.cc

These calls should be updated to include the new std::string&& prefetch_meta parameter to ensure consistency and proper functionality.

Analysis chain

The ZCard method update to accept a std::string&& prefetch_meta parameter is a strategic enhancement for handling metadata more efficiently. Ensure that this change is comprehensively tested, especially in high-load environments where Zset operations are frequent.

The ZCard method has been updated to include a std::string&& prefetch_meta parameter. This change is reflected in the header file src/storage/src/redis.h and in the implementation file src/storage/src/redis_zsets.cc. The next step is to ensure that the method is called with the new parameter across the codebase.

The previous script output shows multiple instances where ZCard is called. However, we need to verify if these calls are updated to include the new std::string&& prefetch_meta parameter.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Ensure `ZCard` is called with the new parameter across the codebase.
rg --type cpp 'ZCard'

Length of output: 4272

Script:

#!/bin/bash
# Description: Verify that the `ZCard` method is called with the new `std::string&& prefetch_meta` parameter across the codebase.
rg --type cpp 'ZCard\(' --context 5

Length of output: 24635

Script:

#!/bin/bash
# Description: Verify the implementation of the `ZCard` method to check if it matches the new signature.
ast-grep --lang cpp --pattern 'Status ZCard(const Slice& key, int32_t* card, std::string&& prefetch_meta) { $$$ }'

Length of output: 116

Status ZCount(const Slice& key, double min, double max, bool left_close, bool right_close, int32_t* ret);
Status ZIncrby(const Slice& key, const Slice& member, double increment, double* ret);
Status ZRange(const Slice& key, int32_t start, int32_t stop, std::vector<ScoreMember>* score_members);
Expand Down Expand Up @@ -323,7 +323,7 @@ class Redis {
Status XAdd(const Slice& key, const std::string& serialized_message, StreamAddTrimArgs& args);
Status XDel(const Slice& key, const std::vector<streamID>& ids, int32_t& count);
Status XTrim(const Slice& key, StreamAddTrimArgs& args, int32_t& count);
Status XRange(const Slice& key, const StreamScanArgs& args, std::vector<IdMessage>& id_messages);
Status XRange(const Slice& key, const StreamScanArgs& args, std::vector<IdMessage>& id_messages, std::string&& prefetch_meta = {});
Status XRevrange(const Slice& key, const StreamScanArgs& args, std::vector<IdMessage>& id_messages);
Status XLen(const Slice& key, int32_t& len);
Status XRead(const StreamReadGroupReadArgs& args, std::vector<std::vector<IdMessage>>& results,
Expand All @@ -333,7 +333,7 @@ class Redis {
rocksdb::ReadOptions& read_options);
// get and parse the stream meta if found
// @return ok only when the stream meta exists
Status GetStreamMeta(StreamMetaValue& tream_meta, const rocksdb::Slice& key, rocksdb::ReadOptions& read_options);
Status GetStreamMeta(StreamMetaValue& tream_meta, const rocksdb::Slice& key, rocksdb::ReadOptions& read_options, std::string&& prefetch_meta = {});

// Before calling this function, the caller should ensure that the ids are valid
Status DeleteStreamMessages(const rocksdb::Slice& key, const StreamMetaValue& stream_meta,
Expand Down
Loading
Loading