Adar Dembo has posted comments on this change.

Change subject: KUDU-1943: Add BlockTransaction to Block Manager
......................................................................


Patch Set 21:

(8 comments)

Almost there, just a few nits left.

Dan, since the implementation has changed quite a bit, do you want to make 
another pass?

http://gerrit.cloudera.org:8080/#/c/7207/21/src/kudu/fs/block_manager-test.cc
File src/kudu/fs/block_manager-test.cc:

PS21, Line 522:   // Disable preallocation for this test as it creates many 
containers.
              :   FLAGS_log_container_preallocate_bytes = 0;
This is no longer true and can be removed, right?


http://gerrit.cloudera.org:8080/#/c/7207/21/src/kudu/fs/file_block_manager.cc
File src/kudu/fs/file_block_manager.cc:

Line 63: DECLARE_bool(block_coalesce_close);
Not needed anymore?


http://gerrit.cloudera.org:8080/#/c/7207/20/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

PS20, Line 883: 
> Done
Should be with a pointer (*), actually.


http://gerrit.cloudera.org:8080/#/c/7207/21/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 79: DECLARE_bool(block_coalesce_close);
Can be removed.


PS21, Line 528: . Marking
, marking


PS21, Line 905: DCHECK
Use DCHECK_EQ


PS21, Line 1054: places
place


PS21, Line 1054: depends
depending


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I01f3ed830cc1f86d7bb8a66b2f780914a88884bd
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

Reply via email to