PragmaTwice commented on code in PR #2402:
URL: https://github.com/apache/kvrocks/pull/2402#discussion_r1894554219


##########
src/storage/redis_db.cc:
##########
@@ -96,7 +96,21 @@ rocksdb::Status Database::GetMetadata(engine::Context &ctx, 
RedisTypes types, co
   auto s = GetRawMetadata(ctx, ns_key, raw_value);
   *rest = *raw_value;
   if (!s.ok()) return s;
-  return ParseMetadataWithStats(types, rest, metadata);
+
+  s = ParseMetadataWithStats(types, rest, metadata);
+  if (!s.ok()) return s;
+
+  // if type is hash, we still need to check if the all of fields expired.
+  if (metadata->Type() == kRedisHash) {
+    HashMetadata hash_metadata(false);
+    s = hash_metadata.Decode(*raw_value);
+    if (!s.ok()) return s;
+    redis::Hash hash_db(storage_, namespace_);
+    if (!hash_db.ExistValidField(ctx, ns_key, hash_metadata)) {
+      return rocksdb::Status::NotFound("no element found");
+    }
+  }

Review Comment:
   It requires at least O(N) IO accesses for every HASH operation. And if you 
think about it: not all operations require to check that all elements are 
expired before the actual operation.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to