kudu git commit: Optionally advance safe time with non-write operations

[gr...@apache.org]

Repository: kudu
Updated Branches:
  refs/heads/branch-1.5.x e1b8444fc -> 611627f2c


Optionally advance safe time with non-write operations

We currently have a problem with safe time advancement when
there are no write messages in the WAL. Because of KUDU-2233,
NO_OP and CHANGE_CONFIG messages are not guaranteed to have
monotonic timestamps, which forced us to ignore them in the
past. This can cause compactions to execute with a timestamp
that is in the beginning of time, causing corruption or crashes.

This patch enables us to advance safe time for all messages
if a flag a set.

Once KUDU-2233 is solved we can flip the flag or remove it
altogether.

Change-Id: I99de2284ba1626581c6e0770c823fd1ba714c1d7
Reviewed-on: http://gerrit.cloudera.org:8080/9396
Tested-by: David Ribeiro Alves <da...@gmail.com>
Reviewed-by: Todd Lipcon <to...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/611627f2
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/611627f2
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/611627f2

Branch: refs/heads/branch-1.5.x
Commit: 611627f2c2e1137f50373954090fa9f7a76badef
Parents: e1b8444
Author: David Alves <dr...@apache.org>
Authored: Mon Jan 8 12:50:00 2018 -0800
Committer: David Ribeiro Alves <da...@gmail.com>
Committed: Thu Feb 22 20:00:38 2018 +0000

----------------------------------------------------------------------
 src/kudu/tablet/tablet_bootstrap.cc | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/611627f2/src/kudu/tablet/tablet_bootstrap.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_bootstrap.cc b/src/kudu/tablet/tablet_bootstrap.cc
index 1eb6533..7479dd3 100644
--- a/src/kudu/tablet/tablet_bootstrap.cc
+++ b/src/kudu/tablet/tablet_bootstrap.cc
@@ -88,6 +88,10 @@
 #include "kudu/util/pb_util.h"
 #include "kudu/util/stopwatch.h"
 
+DEFINE_bool(advance_safe_time_with_non_write_ops, false,
+            "Whether to advance safe time with committed non-write ops like NO_OP "
+            "or CONFIG_CHANGE. This is not totally safe until KUDU-2233 is fixed.");
+TAG_FLAG(advance_safe_time_with_non_write_ops, experimental);
 
 DECLARE_int32(group_commit_queue_size_bytes);
 
@@ -1077,11 +1081,11 @@ Status TabletBootstrap::HandleEntryPair(LogEntryPB* replicate_entry, LogEntryPB*
 
 #undef RETURN_NOT_OK_REPLAY
 
-  // Non-tablet operations should not advance the safe time, because they are
-  // not started serially and so may have timestamps that are out of order.
-  if (op_type == NO_OP || op_type == CHANGE_CONFIG_OP) {
-    return Status::OK();
-  }
+   // Non-tablet operations should not advance the safe time until KUDU-2233 is fixed.
+   if (!FLAGS_advance_safe_time_with_non_write_ops &&
+       (op_type == NO_OP || op_type == CHANGE_CONFIG_OP)) {
+     return Status::OK();
+   }
 
   // Handle safe time advancement:
   //
@@ -1090,8 +1094,10 @@ Status TabletBootstrap::HandleEntryPair(LogEntryPB* replicate_entry, LogEntryPB*
   // safe timestamp to this operation's timestamp.
   //
   // If the hybrid clock is disabled, all transactions will fall into this category.
+  DCHECK(replicate->has_timestamp());
   Timestamp safe_time;
-  if (replicate->write_request().external_consistency_mode() != COMMIT_WAIT) {
+  if (!replicate->write_request().has_external_consistency_mode() ||
+      replicate->write_request().external_consistency_mode() != COMMIT_WAIT) {
     safe_time = Timestamp(replicate->timestamp());
   // ... else we set the safe timestamp to be the transaction's timestamp minus the maximum clock
   // error. This opens the door for problems if the flags changed across reboots, but this is