Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17037 )
Change subject: KUDU-2612 tablet servers automatically register txn participants ...................................................................... Patch Set 12: (10 comments) http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/integration-tests/txn_write_ops-itest.cc File src/kudu/integration-tests/txn_write_ops-itest.cc: http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/integration-tests/txn_write_ops-itest.cc@227 PS12, Line 227: quals nit: incomplete? http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/integration-tests/txn_write_ops-itest.cc@648 PS12, Line 648: CHECK(d); : return d; nit: could combine as return DCHECK_NOTNULL(d) http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/integration-tests/txn_write_ops-itest.cc@1558 PS12, Line 1558: DISABLED_NonTxnWrites Seems this was merged as https://gerrit.cloudera.org/c/17120/? http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/tablet/tablet_replica.h File src/kudu/tablet/tablet_replica.h: http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/tablet/tablet_replica.h@416 PS12, Line 416: write specified nit: specified write operation? http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/tablet/tablet_replica.h@450 PS12, Line 450: respond nit: drop http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/tablet/tablet_replica.h@460 PS12, Line 460: are nit: dropped off? http://gerrit.cloudera.org:8080/#/c/17037/10/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/17037/10/src/kudu/tablet/tablet_replica.cc@633 PS10, Line 633: status_pb_out->set_table_ Seems like this is never anything other than OK? http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/tablet/tablet_replica.cc@1147 PS12, Line 1147: TxnBegin nit: BeginTxn here too? http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/tablet/tablet_replica.cc@1209 PS12, Line 1209: WARN_NOT_OK(self->Submit(), "fail to submit pending txn write requests"); What if this fails with a transient error? Seems like we may fail to submit a write because of memory pressure. If that happens, could we be left stuck with a non-OK inflight_status_, since we don't call Cancel() and UnregisterTxnOpDispatcher()? http://gerrit.cloudera.org:8080/#/c/17037/12/src/kudu/tablet/tablet_replica.cc@1226 PS12, Line 1226: DCHECK(!preliminary_tasks_completed_); Should be done under lock? -- To view, visit http://gerrit.cloudera.org:8080/17037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia383f7afd208c44695c57aab82e3818fa1712ce6 Gerrit-Change-Number: 17037 Gerrit-PatchSet: 12 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 02 Mar 2021 08:38:41 +0000 Gerrit-HasComments: Yes
