You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2021/03/02 08:38:41 UTC
[kudu-CR] KUDU-2612 tablet servers automatically register txn participants
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 <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 02 Mar 2021 08:38:41 +0000
Gerrit-HasComments: Yes