Mike Percy has posted comments on this change.

Change subject: KUDU-236 (part 1). Implement tablet history GC
......................................................................


Patch Set 16:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:

Line 215: 
> some spurious whitespace added
will fix


PS16, Line 387: r
> unused
Done


http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/compaction.cc
File src/kudu/tablet/compaction.cc:

Line 958:             is_garbage_collected = true;
> I don't think this is quite right, because you might hit a REINSERT delta h
Look below at the comment and the DCHECK several lines below. It says it's not 
possible.


Line 1038:       if (is_garbage_collected) {
> think this is better written: if (!is_garbage_collected) output_row_offset+
Will do.


http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/tablet_history_gc-test.cc
File src/kudu/tablet/tablet_history_gc-test.cc:

Line 418:   // Delete every even row.
> per elsewhere, I think this test would catch another bug if you do some of 
see my comment on the other file


-- 
To view, visit http://gerrit.cloudera.org:8080/3076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to