Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16274 )
Change subject: [KUDU-1728] parallelize download blocks in tablet-copy-client ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/16274/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16274/2//COMMIT_MSG@11 PS2, Line 11: Previsouly download blocks from tablet server is sequential nit: Mixed past and present tenses in this sentence. http://gerrit.cloudera.org:8080/#/c/16274/2//COMMIT_MSG@14 PS2, Line 14: Sometimes use only one thread to download those blocks are slow and bandwidth nit: Maybe "sometimes downloading the blocks is slow when only one thread is used and bandwidth isn't the bottleneck" would be clearer? I'm not sure if that's what you meant here. http://gerrit.cloudera.org:8080/#/c/16274/2//COMMIT_MSG@33 PS2, Line 33: Metrics of result is seconds with accuracy of 2 decimal points. What were the numbers before this patch? Also, I'm not sure I understand the benefit of this patch, with tc-8 bd-x doesn't matter and bd-8 is slower than tc-8. Isn't it better to just use tc-8? http://gerrit.cloudera.org:8080/#/c/16274/2/src/kudu/tserver/tablet_copy_client.h File src/kudu/tserver/tablet_copy_client.h: http://gerrit.cloudera.org:8080/#/c/16274/2/src/kudu/tserver/tablet_copy_client.h@285 PS2, Line 285: simple_spinlock mutex_; nit: this shouldn't be called mutex_ if it's actually a spinlock. Also, maybe a more descriptive name would be nice. http://gerrit.cloudera.org:8080/#/c/16274/2/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: http://gerrit.cloudera.org:8080/#/c/16274/2/src/kudu/tserver/tablet_copy_client.cc@714 PS2, Line 714: downloaded_blocks_count + 1 , num_blocks)); nit: why not block_count->Load() + 1? downloaded_blocks_count is used only once. -- To view, visit http://gerrit.cloudera.org:8080/16274 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id83abca7a38cf183d9c27d82bb8a022699079e0e Gerrit-Change-Number: 16274 Gerrit-PatchSet: 2 Gerrit-Owner: wangning <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 03 Aug 2020 10:12:38 +0000 Gerrit-HasComments: Yes
