Skip to content

Commit

Permalink
[#13585] dst: Disable pessimistic locking in clusters with transactio…
Browse files Browse the repository at this point in the history
…n promotion

Summary:
In workloads where transaction promotion is possible, pessimistic locking and deadlock detection may
not work as expected. This revision disables pessimistic locking in such workloads until their
interaction is fixed.

This revision also renames the flag "enable_pessimistic_locking" to "enable_wait_queue_based_pessimistic_locking" for clarity

Test Plan: Jenkins

Reviewers: esheng, pjain

Reviewed By: pjain

Subscribers: sergei, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D19599
  • Loading branch information
robertsami committed Sep 16, 2022
1 parent e676889 commit 5c1941d
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ public class TestPgPessimisticLockingRegress extends BasePgSQLTest {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
flagMap.put("enable_pessimistic_locking", "true");
flagMap.put("enable_wait_queue_based_pessimistic_locking", "true");
flagMap.put("enable_deadlock_detection", "true");
flagMap.put("yb_enable_read_committed_isolation", "false");
flagMap.put("auto_promote_nonlocal_transactions_to_global", "false");
return flagMap;
}

Expand Down
5 changes: 2 additions & 3 deletions src/yb/client/transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ DEFINE_uint64(transaction_heartbeat_usec, 500000 * yb::kTimeMultiplier,
DEFINE_bool(transaction_disable_heartbeat_in_tests, false, "Disable heartbeat during test.");
DECLARE_uint64(max_clock_skew_usec);

DEFINE_bool(auto_promote_nonlocal_transactions_to_global, true,
"Automatically promote transactions touching data outside of region to global.");

DEFINE_test_flag(int32, transaction_inject_flushed_delay_ms, 0,
"Inject delay before processing flushed operations by transaction.");

Expand All @@ -93,6 +90,8 @@ METRIC_DEFINE_counter(server, transaction_promotions,
yb::MetricUnit::kTransactions,
"Number of transactions being promoted to global transactions");

DECLARE_bool(auto_promote_nonlocal_transactions_to_global);

namespace yb {
namespace client {

Expand Down
3 changes: 3 additions & 0 deletions src/yb/common/common_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ DEFINE_bool(enable_deadlock_detection, false, "If true, enables distributed dead
TAG_FLAG(enable_deadlock_detection, advanced);
TAG_FLAG(enable_deadlock_detection, evolving);

DEFINE_bool(auto_promote_nonlocal_transactions_to_global, true,
"Automatically promote transactions touching data outside of region to global.");

DEFINE_test_flag(bool, enable_db_catalog_version_mode, false,
"Enable the per database catalog version mode, a DDL statement is assumed to "
"only affect the current database and will only increment catalog version for "
Expand Down
20 changes: 14 additions & 6 deletions src/yb/tserver/ts_tablet_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,12 @@ DEFINE_test_flag(int32, sleep_after_tombstoning_tablet_secs, 0,
DEFINE_bool(enable_restart_transaction_status_tablets_first, true,
"Set to true to prioritize bootstrapping transaction status tablets first.");

DEFINE_bool(enable_pessimistic_locking, false,
DEFINE_bool(enable_wait_queue_based_pessimistic_locking, false,
"If true, use pessimistic locking behavior in conflict resolution.");
TAG_FLAG(enable_pessimistic_locking, evolving);
TAG_FLAG(enable_pessimistic_locking, hidden);
TAG_FLAG(enable_wait_queue_based_pessimistic_locking, evolving);
TAG_FLAG(enable_wait_queue_based_pessimistic_locking, hidden);

DECLARE_bool(auto_promote_nonlocal_transactions_to_global);

DECLARE_string(rocksdb_compact_flush_rate_limit_sharing_mode);

Expand Down Expand Up @@ -487,9 +489,15 @@ Status TSTabletManager::Init() {
&server_->proxy_cache(),
local_peer_pb_.cloud_info());

if (FLAGS_enable_pessimistic_locking) {
waiting_txn_registry_ = std::make_unique<tablet::LocalWaitingTxnRegistry>(
client_future(), scoped_refptr<server::Clock>(server_->clock()));
if (FLAGS_enable_wait_queue_based_pessimistic_locking) {
if (FLAGS_auto_promote_nonlocal_transactions_to_global) {
LOG(WARNING) << "Ignoring enable_wait_queue_based_pessimistic_locking=true since "
<< "auto_promote_nonlocal_transactions_to_global is enabled. These two features "
<< "are not yet supported together.";
} else {
waiting_txn_registry_ = std::make_unique<tablet::LocalWaitingTxnRegistry>(
client_future(), scoped_refptr<server::Clock>(server_->clock()));
}
}

deque<RaftGroupMetadataPtr> metas;
Expand Down
9 changes: 6 additions & 3 deletions src/yb/yql/pgwrapper/pg_pessimistic_locking-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@

#include "yb/util/pb_util.h"

DECLARE_bool(enable_pessimistic_locking);
DECLARE_bool(enable_wait_queue_based_pessimistic_locking);
DECLARE_bool(enable_deadlock_detection);
DECLARE_bool(TEST_select_all_status_tablets);
DECLARE_string(ysql_pg_conf_csv);
DECLARE_bool(enable_automatic_tablet_splitting);
DECLARE_int32(cleanup_split_tablets_interval_sec);
DECLARE_uint64(rpc_connection_timeout_ms);
DECLARE_bool(auto_promote_nonlocal_transactions_to_global);

using namespace std::literals;

Expand All @@ -58,9 +59,10 @@ class PgPessimisticLockingTest : public PgMiniTestBase {
void SetUp() override {
FLAGS_ysql_pg_conf_csv = Format(
"statement_timeout=$0", kClientStatementTimeoutSeconds * 1ms / 1s);
FLAGS_enable_pessimistic_locking = true;
FLAGS_enable_wait_queue_based_pessimistic_locking = true;
FLAGS_enable_deadlock_detection = true;
FLAGS_TEST_select_all_status_tablets = true;
FLAGS_auto_promote_nonlocal_transactions_to_global = false;
PgMiniTestBase::SetUp();
}

Expand Down Expand Up @@ -665,9 +667,10 @@ class PgTabletSplittingPessimisticLockingTest : public PgTabletSplitTestBase,
protected:
void SetUp() override {
FLAGS_rpc_connection_timeout_ms = 60000;
FLAGS_enable_pessimistic_locking = true;
FLAGS_enable_wait_queue_based_pessimistic_locking = true;
FLAGS_enable_deadlock_detection = true;
FLAGS_enable_automatic_tablet_splitting = false;
FLAGS_auto_promote_nonlocal_transactions_to_global = false;
PgTabletSplitTestBase::SetUp();
}

Expand Down

0 comments on commit 5c1941d

Please sign in to comment.