Skip to content

Commit

Permalink
ENG-2815: Fix TransactionManager destruction
Browse files Browse the repository at this point in the history
Summary:
TransactionManager pointer to implementation becomes null before actual destruction of implementation.
And during this process, it is possible, that RPC callbacks would communicate with TransactionManager.
To address this issue added call to Rpcs::Shutdown, before destroying TransactionManager.

Test Plan: ybd --cxx-test client_ql-transaction-test --gtest_filter QLTransactionTest.ReadRestart -n 100

Reviewers: robert, timur, mikhail

Reviewed By: mikhail

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D4150
  • Loading branch information
spolitov committed Feb 19, 2018
1 parent 655c4f7 commit 8d8030f
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 27 deletions.
70 changes: 43 additions & 27 deletions src/yb/client/ql-transaction-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -312,39 +312,55 @@ TEST_F(QLTransactionTest, Simple) {
ASSERT_OK(cluster_->RestartSync());
}

// Commit flags says whether we should commit write txn during this test.
void QLTransactionTest::TestReadRestart(bool commit) {
google::FlagSaver saver;

FLAGS_max_clock_skew_usec = 250000;

auto write_txn = CreateTransaction();
WriteRows(CreateSession(write_txn));
if (commit) {
ASSERT_OK(write_txn->CommitFuture().get());
}

server::TestClockDeltaChanger delta_changer(-100ms, clock_);
auto txn1 = CreateTransaction();
auto session = CreateSession(txn1);
if (commit) {
for (size_t r = 0; r != kNumRows; ++r) {
auto row = SelectRow(session, KeyForTransactionAndIndex(0, r));
ASSERT_NOK(row);
ASSERT_EQ(ql::ErrorCode::RESTART_REQUIRED, ql::GetErrorCode(row.status()))
<< "Bad row: " << row;
}
auto txn2 = txn1->CreateRestartedTransaction();
session->SetTransaction(txn2);
for (size_t r = 0; r != kNumRows; ++r) {
auto row = SelectRow(session, KeyForTransactionAndIndex(0, r));
ASSERT_OK(row);
ASSERT_EQ(ValueForTransactionAndIndex(0, r, WriteOpType::INSERT), *row);
{
auto write_txn = CreateTransaction();
WriteRows(CreateSession(write_txn));
if (commit) {
ASSERT_OK(write_txn->CommitFuture().get());
}
VerifyData();
} else {
for (size_t r = 0; r != kNumRows; ++r) {
auto row = SelectRow(session, KeyForTransactionAndIndex(0, r));
ASSERT_TRUE(!row.ok() && row.status().IsNotFound()) << "Bad row: " << row;
BOOST_SCOPE_EXIT(write_txn, commit) {
if (!commit) {
write_txn->Abort();
}
} BOOST_SCOPE_EXIT_END;

server::TestClockDeltaChanger delta_changer(-100ms, clock_);
auto txn1 = CreateTransaction();
BOOST_SCOPE_EXIT(txn1, commit) {
if (!commit) {
txn1->Abort();
}
} BOOST_SCOPE_EXIT_END;
auto session = CreateSession(txn1);
if (commit) {
for (size_t r = 0; r != kNumRows; ++r) {
auto row = SelectRow(session, KeyForTransactionAndIndex(0, r));
ASSERT_NOK(row);
ASSERT_EQ(ql::ErrorCode::RESTART_REQUIRED, ql::GetErrorCode(row.status()))
<< "Bad row: " << row;
}
auto txn2 = txn1->CreateRestartedTransaction();
BOOST_SCOPE_EXIT(txn2) {
txn2->Abort();
} BOOST_SCOPE_EXIT_END;
session->SetTransaction(txn2);
for (size_t r = 0; r != kNumRows; ++r) {
auto row = SelectRow(session, KeyForTransactionAndIndex(0, r));
ASSERT_OK(row);
ASSERT_EQ(ValueForTransactionAndIndex(0, r, WriteOpType::INSERT), *row);
}
VerifyData();
} else {
for (size_t r = 0; r != kNumRows; ++r) {
auto row = SelectRow(session, KeyForTransactionAndIndex(0, r));
ASSERT_TRUE(!row.ok() && row.status().IsNotFound()) << "Bad row: " << row;
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/yb/client/transaction_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ class TransactionManager::Impl {
clock_->Update(time);
}

void Shutdown() {
rpcs_.Shutdown();
}

private:
YBClientPtr client_;
scoped_refptr<ClockBase> clock_;
Expand All @@ -169,6 +173,7 @@ TransactionManager::TransactionManager(
: impl_(new Impl(client, clock)) {}

TransactionManager::~TransactionManager() {
impl_->Shutdown();
}

void TransactionManager::PickStatusTablet(PickStatusTabletCallback callback) {
Expand Down

0 comments on commit 8d8030f

Please sign in to comment.