Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7656 )

Change subject: KUDU-2055 [part 1]: Coalesce hole punching when deleting groups 
of blocks
......................................................................


Patch Set 9:

(7 comments)

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

http://gerrit.cloudera.org:8080/#/c/7656/8/src/kudu/fs/block_manager-test.cc@1102
PS8, Line 1102:   
ASSERT_OK(deletion_transaction.CommitDeletedBlocks(&deleted_blocks));
> ASSERT_OK on this?
Done


http://gerrit.cloudera.org:8080/#/c/7656/8/src/kudu/fs/block_manager.h
File src/kudu/fs/block_manager.h:

http://gerrit.cloudera.org:8080/#/c/7656/8/src/kudu/fs/block_manager.h@324
PS8, Line 324: that was seen, if any.
> that was seen, if any
Done


http://gerrit.cloudera.org:8080/#/c/7656/8/src/kudu/fs/block_manager.h@326
PS8, Line 326:     Status first_failure;
> How about calling this "first_failure"?
Done


http://gerrit.cloudera.org:8080/#/c/7656/8/src/kudu/fs/block_manager.h@329
PS8, Line 329: then the block was already deleted.
             :       if (!s.ok()
> then the block was already deleted
Done


http://gerrit.cloudera.org:8080/#/c/7656/8/src/kudu/fs/block_manager.h@339
PS8, Line 339:
> We typically combine statuses by prepending, not appending.
Done


http://gerrit.cloudera.org:8080/#/c/7656/8/src/kudu/fs/block_manager.h@340
PS8, Line 340:
> Don't terminate status messages with periods since they're often combined w
Done


http://gerrit.cloudera.org:8080/#/c/7656/8/src/kudu/fs/block_manager.h@339
PS8, Line 339:                        "first failure",
             :                       
> Should be something like "only deleted $0 blocks, first failure"
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959
Gerrit-Change-Number: 7656
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Thu, 21 Sep 2017 18:45:04 +0000
Gerrit-HasComments: Yes

Reply via email to