Applying https://phabricator.dev.yugabyte.com/D7108 (diff 34457), exc…

…ept the test.
mbautin · Aug 24, 2019 · 3e09352 · 3e09352
1 parent f10e23d
commit 3e09352
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 5 deletions.
diff --git a/src/yb/client/async_rpc.cc b/src/yb/client/async_rpc.cc
@@ -255,6 +255,7 @@ AsyncRpcBase<Req, Resp>::AsyncRpcBase(AsyncRpcData* data, YBConsistencyLevel con
   req_.set_tablet_id(tablet_invoker_.tablet()->tablet_id());
   req_.set_include_trace(IsTracingEnabled());
   const ConsistentReadPoint* read_point = batcher_->read_point();
+  bool has_read_time = false;
   if (read_point) {
     req_.set_propagated_hybrid_time(read_point->Now().ToUint64());
     // Set read time for consistent read only if the table is transaction-enabled and
@@ -263,13 +264,18 @@ AsyncRpcBase<Req, Resp>::AsyncRpcBase(AsyncRpcData* data, YBConsistencyLevel con
         table()->InternalSchema().table_properties().is_transactional()) {
       auto read_time = read_point->GetReadTime(tablet_invoker_.tablet()->tablet_id());
       if (read_time) {
+        has_read_time = true;
         read_time.AddToPB(&req_);
       }
     }
   }
   auto& transaction_metadata = batcher_->transaction_metadata();
   if (!transaction_metadata.transaction_id.is_nil()) {
     SetTransactionMetadata(transaction_metadata, batcher_->may_have_metadata(), &req_);
+    bool serializable = transaction_metadata.isolation == IsolationLevel::SERIALIZABLE_ISOLATION;
+    LOG_IF(DFATAL, has_read_time && serializable)
+        << "Read time should NOT be specified for serializable isolation: "
+        << read_point->GetReadTime().ToString();
   }
   req_.set_memory_limit_score(data->memory_limit_score);
 }
@@ -304,8 +310,13 @@ void AsyncRpcBase<Req, Resp>::SendRpcToTserver() {
   if (!tablet_invoker_.current_ts().HasCapability(CAPABILITY_PickReadTimeAtTabletServer)) {
     ConsistentReadPoint* read_point = batcher_->read_point();
     if (read_point && !read_point->GetReadTime()) {
-      read_point->SetCurrentReadTime();
-      read_point->GetReadTime().AddToPB(&req_);
+      auto txn = batcher_->transaction();
+      // If txn is not set, this is a consistent scan across multiple tablets of a
+      // non-transactional YCQL table.
+      if (!txn || txn->isolation() == IsolationLevel::SNAPSHOT_ISOLATION) {
+        read_point->SetCurrentReadTime();
+        read_point->GetReadTime().AddToPB(&req_);
+      }
     }
   }
 

diff --git a/src/yb/tablet/tablet.cc b/src/yb/tablet/tablet.cc
@@ -1812,9 +1812,14 @@ Status Tablet::StartDocWriteOperation(WriteOperation* operation) {
       if (!read_time) {
         auto safe_time = SafeTime(RequireLease::kTrue);
         read_time = ReadHybridTime::FromHybridTimeRange({safe_time, clock_->NowRange().second});
-      } else if (prepare_result.need_read_snapshot) {
-        DSCHECK_NE(isolation_level, IsolationLevel::SERIALIZABLE_ISOLATION, InvalidArgument,
-                   "Read time should NOT be specified for serializable isolation level");
+      } else if (prepare_result.need_read_snapshot &&
+                 isolation_level == IsolationLevel::SERIALIZABLE_ISOLATION) {
+        auto status = STATUS_FORMAT(
+            InvalidArgument,
+            "Read time should NOT be specified for serializable isolation level: $0",
+            read_time);
+        LOG(DFATAL) << status;
+        return status;
       }
     }
   }