Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19445 )
Change subject: KUDU-1945 Update auto incrementing counter during bootstrap ...................................................................... Patch Set 16: (4 comments) http://gerrit.cloudera.org:8080/#/c/19445/14/src/kudu/integration-tests/auto_incrementing-itest.cc File src/kudu/integration-tests/auto_incrementing-itest.cc: http://gerrit.cloudera.org:8080/#/c/19445/14/src/kudu/integration-tests/auto_incrementing-itest.cc@306 PS14, Line 306: // There could still be a WAL entry with a DELETE OP. : // This might cause the auto incrementing counter to be not set to 0 unless : // FlushDeltaMRS is called. We restart servers to force elections which in turn : // writes more WAL entries which causes these flushes. > Yes, but since there would be only one leader at a given point of time, the Ah, OK -- for some reason I thought the check verification below was comparing the rows from different tablet servers. On a separate note, I recall that presence of DELETES in the WAL doesn't matter -- there isn't a value for the auto-incrementing column in a DELETE op, right? Why to mention the presence of WAL entries with a delete operation then? http://gerrit.cloudera.org:8080/#/c/19445/16/src/kudu/integration-tests/auto_incrementing-itest.cc File src/kudu/integration-tests/auto_incrementing-itest.cc: http://gerrit.cloudera.org:8080/#/c/19445/16/src/kudu/integration-tests/auto_incrementing-itest.cc@294 PS16, Line 294: // We are still yet to GC the wal segments or flush the deletes, : // check back the server after 50ms. nit: a misaligned comment http://gerrit.cloudera.org:8080/#/c/19445/16/src/kudu/integration-tests/auto_incrementing-itest.cc@310 PS16, Line 310: for (int i = 0; i < kNumTabletServers * 3; i++) { Instead, why not just shutdown all tablet servers, and then restart all of them? http://gerrit.cloudera.org:8080/#/c/19445/16/src/kudu/integration-tests/auto_incrementing-itest.cc@311 PS16, Line 311: 3 If doing this fancy stuff, this should be kNumTabletServers, not 3, right? -- 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: 16 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-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sat, 01 Apr 2023 02:42:48 +0000 Gerrit-HasComments: Yes
