Todd Lipcon has posted comments on this change. Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log index ......................................................................
Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/6376/6/src/kudu/consensus/log-test.cc File src/kudu/consensus/log-test.cc: Line 1019: ASSERT_EQ(kSequenceLength, replicates.size()); worth asserting the actual returned opids too, I think http://gerrit.cloudera.org:8080/#/c/6376/6/src/kudu/integration-tests/ts_recovery-itest.cc File src/kudu/integration-tests/ts_recovery-itest.cc: Line 357: // This will cause a crash, but only after they have been written to disk. that's sort of odd. can we rely on this? do we want to? http://gerrit.cloudera.org:8080/#/c/6376/6/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: PS6, Line 789: int64_t overflow = opid->term() - INT32_MIN + 1LL; : CHECK_GE(overflow, 1) << OpIdToString(*opid); : opid->set_term(static_cast<int64_t>(INT32_MAX) + overflow); do we really need to worry about term overflow? I don't think terms can increase at the same rate as indexes -- To view, visit http://gerrit.cloudera.org:8080/6376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
