Wang Xixu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21527 )

Change subject: [Tool] Limit table copying speed
......................................................................


Patch Set 6:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/kudu-tool-test.cc@6011
PS6, Line 6011: 10240
> Are you sure this is small enough to track copying of 20000 rows, especiall
Actually, the data size is 881500. Every batch size is about 5248, total 170 
batches is readout. So, every 2 batches will be block as the 
table_copy_throttler_bytes_per_sec is 10240. That will block for at least 85 
seconds. Please see my next patch: https://gerrit.cloudera.org/c/21609


http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/kudu-tool-test.cc@6029
PS6, Line 6029: client->OpenTable(kNewTableName, &table);
> Wrap this into ASSERT_OK()?
Done


http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/kudu-tool-test.cc@6031
PS6, Line 6031: scanner.Open();
> Wrap this into ASSERT_OK()?
Done


http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/kudu-tool-test.cc@6036
PS6, Line 6036: =
> Shouldn't this be '+=' instead of '='?  With the current code, the assertio
Sorry, that is a mistake.


http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/kudu-tool-test.cc@6038
PS6, Line 6038: must
> nit: must be
Done


http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/table_scanner.h
File src/kudu/tools/table_scanner.h:

http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/table_scanner.h@107
PS6, Line 107: std::shared_ptr
> Why shared_ptr? There isn't any shared ownership for this field, this is so
You are right. Thanks.


http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:

http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/table_scanner.cc@611
PS6, Line 611: copy
> nit: copying
Done


http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/table_scanner.cc@613
PS6, Line 613: SCOPED_LOG_SLOW_EXECUTION
> What's the purpose of this warning?  If the idea was to track cases when it
Done


http://gerrit.cloudera.org:8080/#/c/21527/6/src/kudu/tools/table_scanner.cc@614
PS6, Line 614: 0
> Ah, this 0 is for op tokens, and the 'bytes' consumed is set as necessary,
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37d23f6f5158618f91b67528e152cf2ff4cf38f3
Gerrit-Change-Number: 21527
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Reviewer: Zoltan Martonka <[email protected]>
Gerrit-Comment-Date: Wed, 24 Jul 2024 10:17:46 +0000
Gerrit-HasComments: Yes

Reply via email to