Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/14555 )
Change subject: KUDU-2977 Sharding block map to speed up tserver startup ...................................................................... Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/14555/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14555/1//COMMIT_MSG@11 PS1, Line 11: > Could you summarize this finding and add it to the commit message? Done http://gerrit.cloudera.org:8080/#/c/14555/2/src/kudu/fs/log_block_manager.h File src/kudu/fs/log_block_manager.h: http://gerrit.cloudera.org:8080/#/c/14555/2/src/kudu/fs/log_block_manager.h@409 PS2, Line 409: // Block IDs container used to prevent collisions when creating new anonymous blocks. > Nit: the word "container" is already used elsewhere in the log block manage Done http://gerrit.cloudera.org:8080/#/c/14555/1/src/kudu/fs/log_block_manager.h File src/kudu/fs/log_block_manager.h: http://gerrit.cloudera.org:8080/#/c/14555/1/src/kudu/fs/log_block_manager.h@410 PS1, Line 410: struct ManagedBlockShard { > Why do we need the indirection provided by a unique_ptr? Couldn't we just d For simple_spinlock, it's define by DISALLOW_COPY_AND_ASSIGN(simple_spinlock); For BlockMap, we must construct it's object with BlockAllocator parameter, or we'll get an error like: /home/laiyingchun/kudu_ap/thirdparty/installed/common/include/sparsepp/spp.h:1978:74: error: no matching function for call to ‘kudu::MemTrackerAllocator<std::pair<const kudu::BlockId, scoped_refptr<kudu::fs::internal::LogBlock> > >::MemTrackerAllocator()’ explicit sparsetable(size_type sz = 0, const allocator_type &alloc = allocator_type()) : ... ^~~~~~~~~~~~~~~~ Seems couldn't declare them directly. http://gerrit.cloudera.org:8080/#/c/14555/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/14555/1/src/kudu/fs/log_block_manager.cc@1861 PS1, Line 1861: static const uint64_t kBlockMapMask = kBlockMapChunk - 1; > What was the significance of running on two servers? Different hardware? No much significance. Server 1 has some other loaded processes on it, then I changed to run benchmark on server 2 (also different hardware), sorry to confusing you :( commit message Done. http://gerrit.cloudera.org:8080/#/c/14555/1/src/kudu/fs/log_block_manager.cc@2288 PS1, Line 2288: if (first_failure.ok()) first_failure = s; > Summarize and add to commit message? Done http://gerrit.cloudera.org:8080/#/c/14555/1/src/kudu/fs/log_block_manager.cc@2612 PS1, Line 2612: mem_usage += block_mem; > Same. Done http://gerrit.cloudera.org:8080/#/c/14555/2/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/14555/2/src/kudu/fs/log_block_manager.cc@1916 PS2, Line 1916: // Release all of the memory accounted by the blocks. > When doing something on each shard, could you iterate on the number of elem Done -- To view, visit http://gerrit.cloudera.org:8080/14555 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If0d5c13e051a2c1d6cfd1c9ad7db8a3cd195459d Gerrit-Change-Number: 14555 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Fri, 01 Nov 2019 09:41:09 +0000 Gerrit-HasComments: Yes
