#88: ENG-2982 Ensure we check TTL when we look up the type of a subdoc

Summary:
We had a bug for redis commands which used a combination of EX and NX. While retrieving
only the type of the SubDocument, we wouldn't check the TTL and hence we would surface subdocuments
that shouldn't exist.

This has been fixed to ensure we check the TTL as part of this operation.

Test Plan: Unit test.

Reviewers: mikhail, sergei

Reviewed By: sergei

Subscribers: kannan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D4295

java/yb-jedis-tests/src/test/java/com/yugabyte/jedis/TestYBJedis.java

@@ -242,4 +242,13 @@ public void TestPool() throws Exception {
     assertEquals("v1", jedis.get("k1"));
     jedis.close();
   }
+
+  @Test
+  public void TestNX() throws Exception {
+    assertEquals("OK", jedis_client.set("k1", "v1", "NX", "EX", 5));
+    assertEquals("v1", jedis_client.get("k1"));
+    Thread.sleep(10000);
+    assertEquals("OK", jedis_client.set("k1", "v1", "NX", "EX", 5));
+    assertEquals("v1", jedis_client.get("k1"));
+  }
 }

src/yb/docdb/docdb.cc

@@ -460,7 +460,6 @@ CHECKED_STATUS BuildSubDocument(
           }
         }
       }
-
       // We have found some key that matches our entire subdocument_key, i.e. we didn't skip ahead
       // to a lower level key (with optional object init markers).
       if (IsCollectionType(doc_value.value_type()) ||
@@ -654,7 +653,23 @@ yb::Status GetSubDocument(
 
   if (data.return_type_only) {
     *data.doc_found = doc_value.value_type() != ValueType::kInvalidValueType;
-    *data.result = SubDocument(doc_value.primitive_value());
+    // Check for ttl.
+    if (*data.doc_found) {
+      const MonoDelta ttl = ComputeTTL(doc_value.ttl(), data.table_ttl);
+      DocHybridTime write_time(DocHybridTime::kMin);
+      RETURN_NOT_OK(db_iter->FindLastWriteTime(key_slice, &write_time, nullptr));
+      if (write_time != DocHybridTime::kMin && !ttl.Equals(Value::kMaxTtl)) {
+        const HybridTime expiry =
+            server::HybridClock::AddPhysicalTimeToHybridTime(write_time.hybrid_time(), ttl);
+        if (db_iter->read_time().read.CompareTo(expiry) > 0) {
+          *data.doc_found = false;
+        }
+      }
+    }
+
+    if (*data.doc_found) {
+      *data.result = SubDocument(doc_value.primitive_value());
+    }
     return Status::OK();
   }
 

src/yb/yql/redis/redisserver/redisserver-test.cc

@@ -1242,14 +1242,23 @@ TEST_F(TestRedisService, TestTtl) {
   // Commands are pipelined and only sent when client.commit() is called.
   // sync_commit() waits until all responses are received.
   SyncClient();
-  std::this_thread::sleep_for(std::chrono::seconds(2));
+  std::this_thread::sleep_for(2s);
 
   DoRedisTestBulkString(__LINE__, {"GET", "k1"}, "v1");
   DoRedisTestNull(__LINE__, {"GET", "k2"});
   DoRedisTestBulkString(__LINE__, {"GET", "k3"}, "v3");
   DoRedisTestBulkString(__LINE__, {"GET", "k4"}, "v4");
   DoRedisTestNull(__LINE__, {"GET", "k5"});
 
+  SyncClient();
+  DoRedisTestOk(__LINE__, {"SET", "k10", "v10", "EX", "5", "NX"});
+  SyncClient();
+  DoRedisTestBulkString(__LINE__, {"GET", "k10"}, "v10");
+  SyncClient();
+
+  std::this_thread::sleep_for(10s);
+  DoRedisTestOk(__LINE__, {"SET", "k10", "v10", "EX", "5", "NX"});
+
   SyncClient();
   VerifyCallbacks();
 }