Mailing-List: contact commits-help@kudu.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kudu.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: mpercy@apache.org
To: commits@kudu.apache.org
Date: Wed, 24 Aug 2016 18:48:15 -0000
Message-Id: <a36d73c46c2f483f92c96b03de7a24c6@git.apache.org>
Subject: [1/2] kudu git commit: Various comment / doc improvements
archived-at: Wed, 24 Aug 2016 18:48:17 -0000

Repository: kudu
Updated Branches:
  refs/heads/master cbc865108 -> df3d070e5


Various comment / doc improvements

* Update link to code review for supporting reinsert
* Fix comment in HybridClock header docs
* Improve comment documentation in compaction code

Change-Id: I45d45d265652b0c462aec1bedbcb4b254a2a05da
Reviewed-on: http://gerrit.cloudera.org:8080/3977
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


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

Branch: refs/heads/master
Commit: 535070248eda5ca1483546197ff7d935ff1bd5a4
Parents: cbc8651
Author: Mike Percy <mpercy@apache.org>
Authored: Sun Aug 14 20:11:00 2016 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Wed Aug 24 00:54:08 2016 +0000

----------------------------------------------------------------------
 src/kudu/common/row_changelist.h    |  2 +-
 src/kudu/server/hybrid_clock.h      |  2 +-
 src/kudu/tablet/delta_compaction.cc | 17 ++++++++++-------
 3 files changed, 12 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/53507024/src/kudu/common/row_changelist.h
----------------------------------------------------------------------
diff --git a/src/kudu/common/row_changelist.h b/src/kudu/common/row_changelist.h
index c48af75..480c374 100644
--- a/src/kudu/common/row_changelist.h
+++ b/src/kudu/common/row_changelist.h
@@ -56,7 +56,7 @@ class Schema;
 //   If type == kDelete, then no further data follows. The row is deleted.
 //
 //   If type == kReinsert, then a "tuple-format" row follows. TODO: this will
-//     be changed by http://gerrit.sjc.cloudera.com:8080/#/c/6318/ in the near future.
+//   eventually be changed by something like https://gerrit.cloudera.org/975
 //
 //   If type == kUpdate, then a sequence of column updates follow. Each update
 //   has the format:

http://git-wip-us.apache.org/repos/asf/kudu/blob/53507024/src/kudu/server/hybrid_clock.h
----------------------------------------------------------------------
diff --git a/src/kudu/server/hybrid_clock.h b/src/kudu/server/hybrid_clock.h
index bbf2fcb..1110f9b 100644
--- a/src/kudu/server/hybrid_clock.h
+++ b/src/kudu/server/hybrid_clock.h
@@ -145,7 +145,7 @@ class HybridClock : public Clock {
   static std::string StringifyTimestamp(const Timestamp& timestamp);
 
   // Sets the time to be returned by a mock call to the system clock, for tests.
-  // Requires that 'FLAGS_use_mock_wall_clock' is set to true and that 'now_usec' is less
+  // Requires that 'FLAGS_use_mock_wall_clock' is set to true and that 'now_usec' is higher
   // than the previously set time.
   // NOTE: This refers to the time returned by the system clock, not the time returned
   // by HybridClock, i.e. 'now_usec' is not a HybridTime timestmap and shouldn't have

http://git-wip-us.apache.org/repos/asf/kudu/blob/53507024/src/kudu/tablet/delta_compaction.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/delta_compaction.cc b/src/kudu/tablet/delta_compaction.cc
index 1cd7297..62bca8e 100644
--- a/src/kudu/tablet/delta_compaction.cc
+++ b/src/kudu/tablet/delta_compaction.cc
@@ -127,8 +127,8 @@ Status MajorDeltaCompaction::FlushRowSetAndDeltas() {
     RETURN_NOT_OK(delta_iter_->PrepareBatch(n, DeltaIterator::PREPARE_FOR_COLLECT));
     RETURN_NOT_OK(delta_iter_->CollectMutations(&redo_mutation_block, block.arena()));
 
-    // 3) Apply new UNDO mutations for the current block. The REDO mutations are picked up
-    //    at step 6).
+    // 3) Write new UNDO mutations for the current block. The REDO mutations
+    //    are written out in step 6.
     vector<CompactionInputRow> input_rows;
     input_rows.resize(block.nrows());
     for (int i = 0; i < block.nrows(); i++) {
@@ -142,7 +142,8 @@ Status MajorDeltaCompaction::FlushRowSetAndDeltas() {
       RETURN_NOT_OK(CopyRow(input_row.row, &dst_row, static_cast<Arena*>(nullptr)));
 
       Mutation* new_undos_head = nullptr;
-      // We're ignoring the result from new_redos_head because we'll find them later at step 5).
+      // We're ignoring the result from new_redos_head because we'll find them
+      // later at step 5.
       Mutation* new_redos_head = nullptr;
 
       bool is_garbage_collected;
@@ -176,8 +177,9 @@ Status MajorDeltaCompaction::FlushRowSetAndDeltas() {
     // 4) Write the new base data.
     RETURN_NOT_OK(base_data_writer_->AppendBlock(block));
 
-    // 5) Remove the columns that we're compacting from the delta flush, but keep all the
-    //    delete mutations.
+    // 5) Remove the columns that we've done our major REDO delta compaction on
+    //    from this delta flush, except keep all the delete and reinsert
+    //    mutations.
     arena.Reset();
     vector<DeltaKeyAndUpdate> out;
     RETURN_NOT_OK(delta_iter_->FilterColumnIdsAndCollectDeltas(column_ids_, &out, &arena));
@@ -187,7 +189,8 @@ Status MajorDeltaCompaction::FlushRowSetAndDeltas() {
       RETURN_NOT_OK(OpenRedoDeltaFileWriter());
     }
 
-    // 6) Write the deltas we're not compacting back into a delta file.
+    // 6) Write the remaining REDO deltas that we haven't compacted away back
+    //    into a REDO delta file.
     for (const DeltaKeyAndUpdate& key_and_update : out) {
       RowChangeList update(key_and_update.cell);
       RETURN_NOT_OK_PREPEND(new_redo_delta_writer_->AppendDelta<REDO>(key_and_update.key, update),
@@ -260,7 +263,7 @@ Status MajorDeltaCompaction::Compact() {
     LOG(INFO) << "Preparing to major compact delta file: " << ds->ToString();
   }
 
-  // We defer on calling OpenNewDeltaBlock since we might not need to flush.
+  // We defer calling OpenRedoDeltaFileWriter() since we might not need to flush.
   RETURN_NOT_OK(OpenBaseDataWriter());
   RETURN_NOT_OK(FlushRowSetAndDeltas());
   LOG(INFO) << "Finished major delta compaction of columns " <<