Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19445 )

Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................


Patch Set 4:

(16 comments)

http://gerrit.cloudera.org:8080/#/c/19445/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19445/3//COMMIT_MSG@24
PS3, Line 24: accomplishes this task.
add what are tests added in this patch


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc
File src/kudu/integration-tests/auto_incrementing-itest.cc:

http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@28
PS3, Line 28: #include <utility>
            : #include <vector>
why use <kudu/client/write_op.h> instead of "kudu/client/write_op.h"? Should we 
keep alphabet order?


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@111
PS3, Line 111: lower.
nit: created


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@127
PS3, Line 127:
In InsertData(), we open table before creating new session. But here we create 
new session before opening table. Should we keep the order consistently?


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@174
PS3, Line 174:     resul
nit: indent one less space


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@230
PS3, Line 230:
nit: at least


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@247
PS3, Line 247: nsu
Since loop variable is changed in body, it's better to use a while loop.


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@270
PS3, Line 270: z
nit: Upper case D?


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@308
PS3, Line 308: lete
use while loop


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@349
PS3, Line 349: SERT_OK(clu
Why keeps to update table_uuid in the loop?


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@351
PS3, Line 351:   // Create a table.
break loop if the leader is found


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@361
PS3, Line 361: ->tserv
nit: at least


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@373
PS3, Line 373:
nit: kNumTabletServers - 1


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@385
PS3, Line 385:
nit: have same data


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/integration-tests/auto_incrementing-itest.cc@390
PS3, Line 390: for (int i = 0; i <
use for loop


http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/19445/3/src/kudu/tablet/tablet.cc@451
PS3, Line 451: i
nit: remove space



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19445
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Wed, 22 Mar 2023 08:19:02 +0000
Gerrit-HasComments: Yes

Reply via email to