Adar Dembo has posted comments on this change. Change subject: log block manager: use unsigned int for next_block_id_ ......................................................................
Patch Set 1: (3 comments) I've been wondering whether this deserves a standalone test. I'm inclined to say 'no' since I think the pain is only felt in specific test-only scenarios. Namely: 1) You have to be running in a gtest, so either you're using an InternalMiniCluster or a lower level unit test. 2) You have to be using the block cache singleton. 3) You have to flush some data. 4) You have to restart the block manager some number of times. BTW, KUDU-1538 is related and perhaps should be referenced in the commit description. http://gerrit.cloudera.org:8080/#/c/7796/1//COMMIT_MSG Commit Message: Line 14: This patch changes the type of next_block_id_ to uint64_t to Should probably explain why we're switching it to uint64_t instead of switching every other site to int64_t (since the convention in Kudu is to use int64_t for large integer values). The reason is that block IDs are defined as uint64_t both on disk (fs.proto) and in memory (block_id.h), so it makes more sense to treat them as such in the LBM than to convert them properly. PS1, Line 15: conversation conversion PS1, Line 16: in block id reused. in the reuse of an existing block ID. -- To view, visit http://gerrit.cloudera.org:8080/7796 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib315b20719ef529331304df5c56c4242902524d4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
