PragmaTwice commented on code in PR #1342:
URL: 
https://github.com/apache/incubator-kvrocks/pull/1342#discussion_r1144621512


##########
src/storage/redis_metadata.h:
##########
@@ -97,22 +97,45 @@ class InternalKey {
   bool slot_id_encoded_;
 };
 
+constexpr uint8_t METADATA_64BIT_ENCODING_MASK = 0x80;
+constexpr uint8_t METADATA_TYPE_MASK = 0x0f;
+
 class Metadata {
  public:
+  // metadata flags
+  // <(1-bit) 64bit-common-field-indicator> 0 0 0 <(4-bit) redis-type>
+  // 64bit-common-field-indicator: make `expire` and `size` 64bit instead of 
32bit
+  // NOTE: `expire` is stored in milliseconds for 64bit, seconds for 32bit
+  // redis-type: RedisType for the key-value
   uint8_t flags;
-  int expire;
+
+  // expire timestamp, in milliseconds
+  uint64_t expire;
+
+  // the current version: 53bit timestamp + 11bit counter
   uint64_t version;
-  uint32_t size;
 
- public:
-  explicit Metadata(RedisType type, bool generate_version = true);
+  // element size of the key-value
+  uint64_t size;
+
+  explicit Metadata(RedisType type, bool generate_version = true, bool 
use_64bit_common_field = true);

Review Comment:
   Yes, you are right.



##########
src/storage/redis_metadata.h:
##########
@@ -97,22 +97,45 @@ class InternalKey {
   bool slot_id_encoded_;
 };
 
+constexpr uint8_t METADATA_64BIT_ENCODING_MASK = 0x80;
+constexpr uint8_t METADATA_TYPE_MASK = 0x0f;
+
 class Metadata {
  public:
+  // metadata flags
+  // <(1-bit) 64bit-common-field-indicator> 0 0 0 <(4-bit) redis-type>
+  // 64bit-common-field-indicator: make `expire` and `size` 64bit instead of 
32bit
+  // NOTE: `expire` is stored in milliseconds for 64bit, seconds for 32bit
+  // redis-type: RedisType for the key-value
   uint8_t flags;
-  int expire;
+
+  // expire timestamp, in milliseconds
+  uint64_t expire;
+
+  // the current version: 53bit timestamp + 11bit counter
   uint64_t version;
-  uint32_t size;
 
- public:
-  explicit Metadata(RedisType type, bool generate_version = true);
+  // element size of the key-value
+  uint64_t size;
+
+  explicit Metadata(RedisType type, bool generate_version = true, bool 
use_64bit_common_field = true);

Review Comment:
   > maybe we can set 64-bit encoding in the constructor by default, and it 
will be overwritten by the Decode method in case of decoding from the 'old' 
format?
   
   Yes, you are right.



-- 
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