David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1477. Pending COMMIT message for failed write operation can prevent tablet startup ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/3321/1//COMMIT_MSG Commit Message: Line 12: up due to a pending commit which referred to no mutated stores. ... because all the rows had errors? please clarify http://gerrit.cloudera.org:8080/#/c/3321/1/src/kudu/consensus/log-test-base.h File src/kudu/consensus/log-test-base.h: Line 273: void AppendFailedCommit(const OpId& original_opid) { what's a "FailedCommit" this seems to imply that a we're appending a commit for a "failed" op, whereas from what I understand this is a "successful" op that didn't mutate any stores. http://gerrit.cloudera.org:8080/#/c/3321/1/src/kudu/integration-tests/test_workload.h File src/kudu/integration-tests/test_workload.h: Line 116: void set_write_pattern(WritePattern pattern) { nit: blank line above http://gerrit.cloudera.org:8080/#/c/3321/1/src/kudu/integration-tests/ts_recovery-itest.cc File src/kudu/integration-tests/ts_recovery-itest.cc: Line 148: 1, // TODO TODO what? http://gerrit.cloudera.org:8080/#/c/3321/1/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: Line 271: bool AreAllStoresInactive(const CommitMsg& commit); this is getting super confusing plus we're iterating through the stores multiple times. I suggest that we change this to a single method that returns an enum with values like: NO_STORES_REFERENCED <- Case that you want to address with this patch ALL_STORES_INACTIVE <- Previous case where all stores were flushed. SOME_STORES_ACTIVE <- Case where there is something to do due to unflushed stores. -- To view, visit http://gerrit.cloudera.org:8080/3321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8ecf8d780de1aa89fae4e0510d8291eb1f1cee11 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
