[kudu-CR](branch-1.2.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log index .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: branch-1.2.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](branch-1.2.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log index .. KUDU-1933. consensus: Avoid and repair integer overflow in log index We observed a crash on a long-running master server that looked like the following: F0308 00:25:53.568773 7655 log_index.cc:171] Check failed: log_index > 0 (-2147483648 vs. 0) It turns out that this was caused due to integer overflow on the log index field. This patch fixes this type of truncation in a couple of places (LogReader and MakeOpId()) and adds a couple of new tests that fail without both of those fixes. This patch also adds "repair" logic in log replay during tablet bootstrap that "reverts" integer overflow when it is detected while rewriting the log entry. Finally, this patch includes some test helper fixes to avoid log index integer truncation in future tests. This backport was modified from the original patch by changing the class name of the regression test in log-test.cc to LogTest, which is the only test class available in the 1.2.x code line. Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Reviewed-on: http://gerrit.cloudera.org:8080/6376 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves (cherry picked from commit 8363b74506f8513e2fa9dbf772e30d0abce4e444) Reviewed-on: http://gerrit.cloudera.org:8080/6660 Tested-by: Mike Percy Reviewed-by: Jean-Daniel Cryans --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/log-test-base.h M src/kudu/consensus/log-test.cc M src/kudu/consensus/log_cache-test.cc M src/kudu/consensus/log_reader.cc M src/kudu/consensus/log_reader.h M src/kudu/consensus/opid_util.cc M src/kudu/consensus/opid_util.h M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc 11 files changed, 237 insertions(+), 27 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Mike Percy: Verified -- To view, visit http://gerrit.cloudera.org:8080/6660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: branch-1.2.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy
[kudu-CR](branch-1.2.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index
Mike Percy has posted comments on this change. Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log index .. Patch Set 3: Verified+1 delete_table-test failed due to a known flaky that exists on master. -- To view, visit http://gerrit.cloudera.org:8080/6660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: branch-1.2.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](branch-1.2.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index
Mike Percy has posted comments on this change. Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log index .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/6660/2/src/kudu/consensus/log-test.cc File src/kudu/consensus/log-test.cc: Line 1005: ASSERT_GT(op_id.index(), INT32_MAX); > Should we have this in master too? Sure, I'll submit a patch to add this to master. -- To view, visit http://gerrit.cloudera.org:8080/6660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.2.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR](branch-1.2.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log index .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/6660/2/src/kudu/consensus/log-test.cc File src/kudu/consensus/log-test.cc: Line 1005: ASSERT_GT(op_id.index(), INT32_MAX); Should we have this in master too? -- To view, visit http://gerrit.cloudera.org:8080/6660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.2.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR](branch-1.2.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index
Hello David Ribeiro Alves, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6660 to look at the new patch set (#2). Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log index .. KUDU-1933. consensus: Avoid and repair integer overflow in log index We observed a crash on a long-running master server that looked like the following: F0308 00:25:53.568773 7655 log_index.cc:171] Check failed: log_index > 0 (-2147483648 vs. 0) It turns out that this was caused due to integer overflow on the log index field. This patch fixes this type of truncation in a couple of places (LogReader and MakeOpId()) and adds a couple of new tests that fail without both of those fixes. This patch also adds "repair" logic in log replay during tablet bootstrap that "reverts" integer overflow when it is detected while rewriting the log entry. Finally, this patch includes some test helper fixes to avoid log index integer truncation in future tests. This backport was modified from the original patch by changing the class name of the regression test in log-test.cc to LogTest, which is the only test class available in the 1.2.x code line. Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Reviewed-on: http://gerrit.cloudera.org:8080/6376 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves (cherry picked from commit 8363b74506f8513e2fa9dbf772e30d0abce4e444) --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/log-test-base.h M src/kudu/consensus/log-test.cc M src/kudu/consensus/log_cache-test.cc M src/kudu/consensus/log_reader.cc M src/kudu/consensus/log_reader.h M src/kudu/consensus/opid_util.cc M src/kudu/consensus/opid_util.h M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc 11 files changed, 237 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/6660/2 -- To view, visit http://gerrit.cloudera.org:8080/6660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.2.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.2.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index
Hello David Ribeiro Alves, Kudu Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/6660 to review the following change. Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log index .. KUDU-1933. consensus: Avoid and repair integer overflow in log index We observed a crash on a long-running master server that looked like the following: F0308 00:25:53.568773 7655 log_index.cc:171] Check failed: log_index > 0 (-2147483648 vs. 0) It turns out that this was caused due to integer overflow on the log index field. This patch fixes this type of truncation in a couple of places (LogReader and MakeOpId()) and adds a couple of new tests that fail without both of those fixes. This patch also adds "repair" logic in log replay during tablet bootstrap that "reverts" integer overflow when it is detected while rewriting the log entry. Finally, this patch includes some test helper fixes to avoid log index integer truncation in future tests. Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Reviewed-on: http://gerrit.cloudera.org:8080/6376 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves (cherry picked from commit 8363b74506f8513e2fa9dbf772e30d0abce4e444) --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/log-test-base.h M src/kudu/consensus/log-test.cc M src/kudu/consensus/log_cache-test.cc M src/kudu/consensus/log_reader.cc M src/kudu/consensus/log_reader.h M src/kudu/consensus/opid_util.cc M src/kudu/consensus/opid_util.h M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc 11 files changed, 236 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/6660/1 -- To view, visit http://gerrit.cloudera.org:8080/6660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.2.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
