From 82f00f5179720c8cee6cd650763d184ba943be92 Mon Sep 17 00:00:00 2001
From: Moti Cohen <moticless@gmail.com>
Date: Wed, 24 Jul 2024 08:39:10 +0300
Subject: [PATCH] Optimize RDB_TYPE_HASH_METADATA to keep relative expiration
 time (#13438)

Modify RDB_TYPE_HASH_METADATA layout to store expiration times relative
to the minimum expiration time, which is written at the start as absolute time.
---
 src/rdb.c | 48 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/rdb.c b/src/rdb.c
index 04ffb3a4c..2a39c8af5 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -699,7 +699,7 @@ int rdbSaveObjectType(rio *rdb, robj *o) {
         else if (o->encoding == OBJ_ENCODING_LISTPACK_EX)
             return rdbSaveType(rdb,RDB_TYPE_HASH_LISTPACK_EX);
         else if (o->encoding == OBJ_ENCODING_HT) {
-            if (hashTypeGetMinExpire(o, 0) == EB_EXPIRE_TIME_INVALID)
+            if (hashTypeGetMinExpire(o, /*accurate*/ 1) == EB_EXPIRE_TIME_INVALID)
                 return rdbSaveType(rdb,RDB_TYPE_HASH);
             else
                 return rdbSaveType(rdb,RDB_TYPE_HASH_METADATA);
@@ -970,7 +970,7 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) {
              * RDB_TYPE_HASH_METADATA layout, including tuples of [ttl][field][value].
              * Otherwise, use the standard RDB_TYPE_HASH layout containing only
              * the tuples [field][value]. */
-            uint64_t minExpire = hashTypeGetMinExpire(o, 0);
+            uint64_t minExpire = hashTypeGetMinExpire(o, 1);
 
             /* if RDB_TYPE_HASH_METADATA (Can have TTLs on fields) */
             if (minExpire != EB_EXPIRE_TIME_INVALID) {
@@ -996,9 +996,13 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) {
 
                 /* save the TTL */
                 if (hashWithMeta) {
-                    uint64_t ttl = hfieldGetExpireTime(field);
-                    /* 0 is used to indicate no TTL is set for this field */
-                    if (ttl == EB_EXPIRE_TIME_INVALID) ttl = 0;
+                    uint64_t ttl, expiryTime= hfieldGetExpireTime(field);
+
+                    /* Saved TTL value:
+                     *  - 0: Indicates no TTL. This is common case so we keep it small.
+                     *  - Otherwise: TTL is relative to minExpire (with +1 to avoid 0 that already taken)
+                     */
+                    ttl = (expiryTime == EB_EXPIRE_TIME_INVALID) ? 0 : expiryTime - minExpire + 1;
                     if ((n = rdbSaveLen(rdb, ttl)) == -1) {
                         dictReleaseIterator(di);
                         return -1;
@@ -2261,19 +2265,24 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error)
     } else if (rdbtype == RDB_TYPE_HASH_METADATA || rdbtype == RDB_TYPE_HASH_METADATA_PRE_GA) {
         sds value;
         hfield field;
-        uint64_t expireAt;
+        uint64_t ttl, expireAt, minExpire = EB_EXPIRE_TIME_INVALID;
         dict *dupSearchDict = NULL;
 
-        /* If hash with TTLs, load next/min expiration time */
+        /* If hash with TTLs, load next/min expiration time
+         *
+         * - This value is serialized for future use-case of streaming the object
+         *   directly to FLASH (while keeping in mem its next expiration time).
+         * - It is also being used to keep only relative TTL for fields in RDB file.
+         */
         if (rdbtype == RDB_TYPE_HASH_METADATA) {
-            uint64_t minExpire = rdbLoadMillisecondTime(rdb, RDB_VERSION);
-            /* This value was serialized for future use-case of streaming the object
-             * directly to FLASH (while keeping in mem its next expiration time) */
-            UNUSED(minExpire);
+            minExpire = rdbLoadMillisecondTime(rdb, RDB_VERSION);
             if (rioGetReadError(rdb)) {
                 rdbReportCorruptRDB("Hash failed loading minExpire");
                 return NULL;
             }
+            if (minExpire > EB_EXPIRE_TIME_INVALID) {
+                rdbReportCorruptRDB("Hash read invalid minExpire value");
+            }
         }
 
         len = rdbLoadLen(rdb, NULL);
@@ -2301,14 +2310,27 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error)
             len--;
 
             /* read the TTL */
-            if (rdbLoadLenByRef(rdb, NULL, &expireAt) == -1) {
+            if (rdbLoadLenByRef(rdb, NULL, &ttl) == -1) {
                 serverLog(LL_WARNING, "failed reading hash TTL");
                 decrRefCount(o);
                 if (dupSearchDict != NULL) dictRelease(dupSearchDict);
                 return NULL;
             }
+
+
+            if (rdbtype == RDB_TYPE_HASH_METADATA) {
+                /* Loaded TTL value:
+                 *  - 0: Indicates no TTL. This is common case so we keep it small.
+                 *  - Otherwise: TTL is relative to minExpire (with +1 to avoid 0 that already taken)
+                 */
+                expireAt = (ttl != 0) ? (ttl + minExpire - 1) : 0;
+            } else { /* RDB_TYPE_HASH_METADATA_PRE_GA */
+                expireAt = ttl; /* Value is absolute */
+            }
+
             if (expireAt > EB_EXPIRE_TIME_MAX) {
-                rdbReportCorruptRDB("invalid expireAt time: %llu", (unsigned long long)expireAt);
+                rdbReportCorruptRDB("invalid expireAt time: %llu",
+                                    (unsigned long long) expireAt);
                 decrRefCount(o);
                 return NULL;
             }