Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12462 )
Change subject: KUDU-2690: don't roll log schema on failed alter ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/12462/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12462/6//COMMIT_MSG@25 PS6, Line 25: use uses http://gerrit.cloudera.org:8080/#/c/12462/6/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: http://gerrit.cloudera.org:8080/#/c/12462/6/src/kudu/tablet/tablet_bootstrap.cc@1458 PS6, Line 1458: // 3. The commit message contains a 'result', which should only happen if the : // alter resulted in a failure. Exit out without attempting the alter. AlterSchemas aren't very common, so maybe we should follow the convention of other REPLICATE messages and put a result in even on success? AFAICT this is what we do for Write; do we do the same for ChangeConfig? http://gerrit.cloudera.org:8080/#/c/12462/6/src/kudu/tablet/tablet_replica-test.cc File src/kudu/tablet/tablet_replica-test.cc: http://gerrit.cloudera.org:8080/#/c/12462/6/src/kudu/tablet/tablet_replica-test.cc@137 PS6, Line 137: scoped_refptr<ConsensusMetadataManager> cmeta_manager( Could we store this so that your new test needn't recreate it as part of the test? http://gerrit.cloudera.org:8080/#/c/12462/2/src/kudu/tablet/transactions/alter_schema_transaction.cc File src/kudu/tablet/transactions/alter_schema_transaction.cc: http://gerrit.cloudera.org:8080/#/c/12462/2/src/kudu/tablet/transactions/alter_schema_transaction.cc@153 PS2, Line 153: } : : // The schema lock was acquired by Tablet::CreatePreparedAlterSchem > Nothing has changed, so I'm inclined to say no. That said, I'm not sure if I'm wondering, if this is skipped, whether a client waiting on an ongoing AlterTable operation will be notified of its completion. Tablet.AlterSchema succeeded, which means the metadata's schema_version_ was updated, which would make it into the next heartbeat if a full heartbeat was scheduled. But without marking the tablet as dirty, will that happen? If so, why? -- To view, visit http://gerrit.cloudera.org:8080/12462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id761851741297e29a4666bec0c34fc4f7285f715 Gerrit-Change-Number: 12462 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 19 Feb 2019 20:11:18 +0000 Gerrit-HasComments: Yes
