This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 0ab94e91936d6b41f730e9b38c28c4bff44925c2 Author: Alexey Serbin <[email protected]> AuthorDate: Sat Jan 23 09:45:17 2021 -0800 [tablet] OpDriver::ExecuteAsync() return void This patch addresses a long standing TODO in OpDriver::ExecuteAsync(), changing its signature to return void instead of Status. This patch doesn't contain any functional modifications. Change-Id: I459cdac9f17a6345b2508a6b7137c0768811dd87 Reviewed-on: http://gerrit.cloudera.org:8080/16973 Reviewed-by: Andrew Wong <[email protected]> Reviewed-by: Mahesh Reddy <[email protected]> Tested-by: Alexey Serbin <[email protected]> --- src/kudu/tablet/ops/op_driver.cc | 7 ++----- src/kudu/tablet/ops/op_driver.h | 7 +++---- src/kudu/tablet/tablet_replica-test.cc | 3 +-- src/kudu/tablet/tablet_replica.cc | 11 +++++++---- src/kudu/tablet/txn_participant-test.cc | 2 +- 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/kudu/tablet/ops/op_driver.cc b/src/kudu/tablet/ops/op_driver.cc index eb8f630..82b0cbe 100644 --- a/src/kudu/tablet/ops/op_driver.cc +++ b/src/kudu/tablet/ops/op_driver.cc @@ -209,7 +209,7 @@ string OpDriver::ToStringUnlocked() const { } -Status OpDriver::ExecuteAsync() { +void OpDriver::ExecuteAsync() { VLOG_WITH_PREFIX(4) << "ExecuteAsync()"; TRACE_EVENT_FLOW_BEGIN0("op", "ExecuteAsync", this); ADOPT_TRACE(trace()); @@ -225,12 +225,9 @@ Status OpDriver::ExecuteAsync() { s = prepare_pool_token_->Submit([this]() { this->PrepareTask(); }); } - if (!s.ok()) { + if (PREDICT_FALSE(!s.ok())) { HandleFailure(s); } - - // TODO: make this return void - return Status::OK(); } void OpDriver::PrepareTask() { diff --git a/src/kudu/tablet/ops/op_driver.h b/src/kudu/tablet/ops/op_driver.h index ac98787..8b58c11 100644 --- a/src/kudu/tablet/ops/op_driver.h +++ b/src/kudu/tablet/ops/op_driver.h @@ -241,10 +241,9 @@ class OpDriver : public RefCountedThreadSafe<OpDriver> { // be used in tight loops. consensus::OpId GetOpId(); - // Submits the op for execution. - // The returned status acknowledges any error on the submission process. - // The op will be replied to asynchronously. - Status ExecuteAsync(); + // Submits the op for execution. Any errors on the submission process are + // handled by this method itself. The op will be replied to asynchronously. + void ExecuteAsync(); // Aborts the op, if possible. Since ops are executed in // multiple stages by multiple executors it might not be possible to stop diff --git a/src/kudu/tablet/tablet_replica-test.cc b/src/kudu/tablet/tablet_replica-test.cc index 32a2c62..db077bd 100644 --- a/src/kudu/tablet/tablet_replica-test.cc +++ b/src/kudu/tablet/tablet_replica-test.cc @@ -426,8 +426,7 @@ TEST_F(TabletReplicaTest, TestActiveOpPreventsLogGC) { scoped_refptr<OpDriver> driver; ASSERT_OK(tablet_replica_->NewLeaderOpDriver(std::move(op), &driver)); - - ASSERT_OK(driver->ExecuteAsync()); + driver->ExecuteAsync(); apply_started.Wait(); ASSERT_TRUE(driver->GetOpId().IsInitialized()) << "By the time an op is applied, it should have an Opid"; diff --git a/src/kudu/tablet/tablet_replica.cc b/src/kudu/tablet/tablet_replica.cc index 682d9e8..d40a8c5 100644 --- a/src/kudu/tablet/tablet_replica.cc +++ b/src/kudu/tablet/tablet_replica.cc @@ -442,7 +442,8 @@ Status TabletReplica::SubmitWrite(unique_ptr<WriteOpState> op_state) { unique_ptr<WriteOp> op(new WriteOp(std::move(op_state), consensus::LEADER)); scoped_refptr<OpDriver> driver; RETURN_NOT_OK(NewLeaderOpDriver(std::move(op), &driver)); - return driver->ExecuteAsync(); + driver->ExecuteAsync(); + return Status::OK(); } Status TabletReplica::SubmitTxnParticipantOp(std::unique_ptr<ParticipantOpState> op_state) { @@ -452,7 +453,8 @@ Status TabletReplica::SubmitTxnParticipantOp(std::unique_ptr<ParticipantOpState> unique_ptr<ParticipantOp> op(new ParticipantOp(std::move(op_state), consensus::LEADER)); scoped_refptr<OpDriver> driver; RETURN_NOT_OK(NewLeaderOpDriver(std::move(op), &driver)); - return driver->ExecuteAsync(); + driver->ExecuteAsync(); + return Status::OK(); } Status TabletReplica::SubmitAlterSchema(unique_ptr<AlterSchemaOpState> state) { @@ -462,7 +464,8 @@ Status TabletReplica::SubmitAlterSchema(unique_ptr<AlterSchemaOpState> state) { new AlterSchemaOp(std::move(state), consensus::LEADER)); scoped_refptr<OpDriver> driver; RETURN_NOT_OK(NewLeaderOpDriver(std::move(op), &driver)); - return driver->ExecuteAsync(); + driver->ExecuteAsync(); + return Status::OK(); } void TabletReplica::GetTabletStatusPB(TabletStatusPB* status_pb_out) const { @@ -695,7 +698,7 @@ Status TabletReplica::StartFollowerOp(const scoped_refptr<ConsensusRound>& round state->consensus_round()->SetConsensusReplicatedCallback( [driver_raw](const Status& s) { driver_raw->ReplicationFinished(s); }); - RETURN_NOT_OK(driver->ExecuteAsync()); + driver->ExecuteAsync(); return Status::OK(); } diff --git a/src/kudu/tablet/txn_participant-test.cc b/src/kudu/tablet/txn_participant-test.cc index 5507892..6a8fd74 100644 --- a/src/kudu/tablet/txn_participant-test.cc +++ b/src/kudu/tablet/txn_participant-test.cc @@ -798,7 +798,7 @@ TEST_F(TxnParticipantTest, TestActiveParticipantOpsAnchorWALs) { unique_ptr<DelayedParticipantOp> op( new DelayedParticipantOp(&apply_start, &apply_continue, std::move(op_state))); ASSERT_OK(tablet_replica_->NewLeaderOpDriver(std::move(op), &driver)); - ASSERT_OK(driver->ExecuteAsync()); + driver->ExecuteAsync(); // Wait for the apply to start, indicating that we have persisted and // replicated but not yet Raft committed the participant op. apply_start.Wait();
