Skip to content

Commit

Permalink
Revert "[BACKPORT 2.12][#12775] YSQL: Show transaction priority of th…
Browse files Browse the repository at this point in the history 
…e active transaction in current session"

Summary:
When `0a12cb7ff08cd0afbc29986c62c5481385510097` was backported to 2.12, bug was introduced when backporting: we try to access txn's priority using txn_->GetPriority();, even when txn_ might be null. This results in a crash when trying to access the priority of such nonexistent txn (see #13348 for details).

This reverts commit 99ffb31.

Test Plan: jenkins: rebase 2.12.9

Reviewers: smishra, pjain

Reviewed By: smishra, pjain

Differential Revision: https://phabricator.dev.yugabyte.com/D18418
  • Loading branch information
@lnguyen-yugabyte
lnguyen-yugabyte committed Jul 19, 2022
1 parent 24ab44d commit 7cc503b
Show file tree
Hide file tree
Showing 11 changed files with 9 additions and 197 deletions.
36 changes: 0 additions & 36 deletions src/postgres/src/backend/utils/misc/guc.c
View file
Expand Up @@ -133,7 +133,6 @@ extern bool optimize_bounded_sort;

static double yb_transaction_priority_lower_bound = 0.0;
static double yb_transaction_priority_upper_bound = 1.0;
static double yb_transaction_priority = 0.0;

static int GUC_check_errcode_value;

Expand Down Expand Up @@ -210,9 +209,6 @@ static bool check_transaction_priority_lower_bound(double *newval, void **extra,
extern void YBCAssignTransactionPriorityLowerBound(double newval, void* extra);
static bool check_transaction_priority_upper_bound(double *newval, void **extra, GucSource source);
extern void YBCAssignTransactionPriorityUpperBound(double newval, void* extra);
extern double YBCGetTransactionPriority();
extern TxnPriorityRequirement YBCGetTransactionPriorityType();
static const char *show_transaction_priority(void);

static void assign_ysql_upgrade_mode(bool newval, void *extra);

Expand Down Expand Up @@ -3611,16 +3607,6 @@ static struct config_real ConfigureNamesReal[] =
1.0, 0.0, 1.0,
check_transaction_priority_upper_bound, YBCAssignTransactionPriorityUpperBound, NULL
},
{
{"yb_transaction_priority", PGC_INTERNAL, CLIENT_CONN_STATEMENT,
gettext_noop("Gets the transaction priority used by the current active "
"transaction in the session. If no transaction is active, return 0"),
NULL
},
&yb_transaction_priority,
0.0, 0.0, 1.0,
NULL, NULL, show_transaction_priority
},

{
{"retry_backoff_multiplier", PGC_USERSET, CLIENT_CONN_STATEMENT,
Expand Down Expand Up @@ -11727,28 +11713,6 @@ check_transaction_priority_upper_bound(double *newval, void **extra, GucSource s
return true;
}

static const char *
show_transaction_priority(void)
{
TxnPriorityRequirement txn_priority_type;
double txn_priority;
static char buf[50];

txn_priority_type = YBCGetTransactionPriorityType();
txn_priority = YBCGetTransactionPriority();

if (txn_priority_type == kHighestPriority)
snprintf(buf, sizeof(buf), "Highest priority transaction");
else if (txn_priority_type == kHigherPriorityRange)
snprintf(buf, sizeof(buf),
"%.9lf (High priority transaction)", txn_priority);
else
snprintf(buf, sizeof(buf),
"%.9lf (Normal priority transaction)", txn_priority);

return buf;
}

static void
assign_ysql_upgrade_mode(bool newval, void *extra)
{
Expand Down
65 changes: 3 additions & 62 deletions src/postgres/src/test/regress/expected/yb_guc.out
View file
Expand Up @@ -121,65 +121,6 @@ EXPLAIN SELECT j FROM test_scan;
Seq Scan on test_scan (cost=10000000000.00..10000000100.00 rows=1000 width=4)
(1 row)

-- Show transaction priority. As it is not possible to have a deterministic
-- yb_transaction_priority, we set yb_transaction_priority_lower_bound and
-- yb_transaction_priority_upper_bound to be the same, which forces
-- yb_transaction_priority to be equal to those two.
set yb_transaction_priority_lower_bound = 0.4;
set yb_transaction_priority_upper_bound = 0.4;
BEGIN TRANSACTION;
INSERT INTO test_scan (i, j) values (1, 1), (2, 2), (3, 3);
show yb_transaction_priority;
yb_transaction_priority
-------------------------------------------
0.400000000 (Normal priority transaction)
(1 row)

COMMIT;
-- Trying to set yb_transaction_priority will be an error
set yb_transaction_priority = 0.3; -- ERROR
ERROR: parameter "yb_transaction_priority" cannot be changed
-- High priority transaction
set yb_transaction_priority_lower_bound = 0.4;
set yb_transaction_priority_upper_bound = 0.4;
BEGIN TRANSACTION;
SELECT i, j FROM test_scan WHERE i = 1 FOR UPDATE;
i | j
---+---
1 | 1
(1 row)

show yb_transaction_priority;
yb_transaction_priority
-----------------------------------------
0.400000000 (High priority transaction)
(1 row)

COMMIT;
-- Highest priority transaction
set yb_transaction_priority_upper_bound = 1;
set yb_transaction_priority_lower_bound = 1;
BEGIN TRANSACTION;
SELECT i, j FROM test_scan WHERE i = 1 FOR UPDATE;
i | j
---+---
1 | 1
(1 row)

show yb_transaction_priority;
yb_transaction_priority
------------------------------
Highest priority transaction
(1 row)

COMMIT;
-- Showing yb_transaction_priority outside a transaction block
show yb_transaction_priority;
yb_transaction_priority
-------------------------------------------
0.000000000 (Normal priority transaction)
(1 row)

-- SET LOCAL is restricted by a function SET option
create or replace function myfunc(int) returns text as $$
begin
Expand Down Expand Up @@ -210,21 +151,21 @@ SHOW plpgsql.extra_foo_warnings; -- but the parameter is set
-- test `yb_db_admin` role can set and reset yb_db_admin-allowed PGC_SUSET variables
SET SESSION AUTHORIZATION yb_db_admin;
SHOW session_replication_role;
session_replication_role
session_replication_role
--------------------------
origin
(1 row)

SET session_replication_role TO replica;
SHOW session_replication_role;
session_replication_role
session_replication_role
--------------------------
replica
(1 row)

RESET session_replication_role;
SHOW session_replication_role;
session_replication_role
session_replication_role
--------------------------
origin
(1 row)
Expand Down
33 changes: 0 additions & 33 deletions src/postgres/src/test/regress/sql/yb_guc.sql
View file
Expand Up @@ -53,39 +53,6 @@ EXPLAIN SELECT j FROM test_scan;
set enable_indexonlyscan = off;
EXPLAIN SELECT j FROM test_scan;

-- Show transaction priority. As it is not possible to have a deterministic
-- yb_transaction_priority, we set yb_transaction_priority_lower_bound and
-- yb_transaction_priority_upper_bound to be the same, which forces
-- yb_transaction_priority to be equal to those two.
set yb_transaction_priority_lower_bound = 0.4;
set yb_transaction_priority_upper_bound = 0.4;
BEGIN TRANSACTION;
INSERT INTO test_scan (i, j) values (1, 1), (2, 2), (3, 3);
show yb_transaction_priority;
COMMIT;

-- Trying to set yb_transaction_priority will be an error
set yb_transaction_priority = 0.3; -- ERROR

-- High priority transaction
set yb_transaction_priority_lower_bound = 0.4;
set yb_transaction_priority_upper_bound = 0.4;
BEGIN TRANSACTION;
SELECT i, j FROM test_scan WHERE i = 1 FOR UPDATE;
show yb_transaction_priority;
COMMIT;

-- Highest priority transaction
set yb_transaction_priority_upper_bound = 1;
set yb_transaction_priority_lower_bound = 1;
BEGIN TRANSACTION;
SELECT i, j FROM test_scan WHERE i = 1 FOR UPDATE;
show yb_transaction_priority;
COMMIT;

-- Showing yb_transaction_priority outside a transaction block
show yb_transaction_priority;

-- SET LOCAL is restricted by a function SET option
create or replace function myfunc(int) returns text as $$
begin
Expand Down
6 changes: 6 additions & 0 deletions src/yb/client/transaction.h
View file
Expand Up @@ -36,6 +36,12 @@ class HybridTime;

class Trace;

enum TxnPriorityRequirement {
kLowerPriorityRange,
kHigherPriorityRange,
kHighestPriority
};

namespace client {

using Waiter = boost::function<void(const Status&)>;
Expand Down
37 changes: 0 additions & 37 deletions src/yb/yql/pggate/pg_txn_manager.cc
View file
Expand Up @@ -87,19 +87,6 @@ uint64_t ConvertHighPriorityTxnBound(double value) {
return ConvertBound(value, yb::kHighPriTxnLowerBound, yb::kHighPriTxnUpperBound);
}
// Convert uint64_t value in range [minValue, maxValue] to double value in range 0..1
double ToTxnPriority(uint64_t value, uint64_t minValue, uint64_t maxValue) {
if (value <= minValue) {
return 0.0;
}
if (value >= maxValue) {
return 1.0;
}
return static_cast<double>(value - minValue) / (maxValue - minValue);
}
} // namespace
extern "C" {
Expand Down Expand Up @@ -587,29 +574,5 @@ std::string PgTxnManager::TxnStateDebugStr() const {
pg_isolation_level);
}
double PgTxnManager::GetTransactionPriority() const {
auto priority = txn_->GetPriority();
if (priority <= yb::kRegularTxnUpperBound) {
return ToTxnPriority(priority,
yb::kRegularTxnLowerBound,
yb::kRegularTxnUpperBound);
}
return ToTxnPriority(priority,
yb::kHighPriTxnLowerBound,
yb::kHighPriTxnUpperBound);
}
TxnPriorityRequirement PgTxnManager::GetTransactionPriorityType() const {
auto priority = txn_->GetPriority();
if (priority <= yb::kRegularTxnUpperBound) {
return kLowerPriorityRange;
}
if (priority < yb::kHighPriTxnUpperBound) {
return kHigherPriorityRange;
}
return kHighestPriority;
}
} // namespace pggate
} // namespace yb
3 changes: 0 additions & 3 deletions src/yb/yql/pggate/pg_txn_manager.h
View file
Expand Up @@ -95,9 +95,6 @@ class PgTxnManager : public RefCountedThreadSafe<PgTxnManager> {
bool IsTxnInProgress() const { return txn_in_progress_; }
bool ShouldUseFollowerReads() const { return updated_read_time_for_follower_reads_; }

double GetTransactionPriority() const;
TxnPriorityRequirement GetTransactionPriorityType() const;

private:
YB_STRONGLY_TYPED_BOOL(NeedsHigherPriorityTxn);
YB_STRONGLY_TYPED_BOOL(SavePriority);
Expand Down
8 changes: 0 additions & 8 deletions src/yb/yql/pggate/pggate.cc
View file
Expand Up @@ -1532,14 +1532,6 @@ Status PgApiImpl::RollbackSubTransaction(SubTransactionId id) {
return pg_txn_manager_->RollbackSubTransaction(id);
}

double PgApiImpl::GetTransactionPriority() const {
return pg_txn_manager_->GetTransactionPriority();
}

TxnPriorityRequirement PgApiImpl::GetTransactionPriorityType() const {
return pg_txn_manager_->GetTransactionPriorityType();
}

void PgApiImpl::ResetCatalogReadTime() {
pg_session_->ResetCatalogReadPoint();
}
Expand Down
2 changes: 0 additions & 2 deletions src/yb/yql/pggate/pggate.h
View file
Expand Up @@ -501,8 +501,6 @@ class PgApiImpl {
void ClearSeparateDdlTxnMode();
CHECKED_STATUS SetActiveSubTransaction(SubTransactionId id);
CHECKED_STATUS RollbackSubTransaction(SubTransactionId id);
double GetTransactionPriority() const;
TxnPriorityRequirement GetTransactionPriorityType() const;

//------------------------------------------------------------------------------------------------
// Expressions.
Expand Down
6 changes: 0 additions & 6 deletions src/yb/yql/pggate/ybc_pg_typedefs.h
View file
Expand Up @@ -179,12 +179,6 @@ typedef enum PgDatumKind {
YB_YQL_DATUM_LIMIT_MIN,
} YBCPgDatumKind;

typedef enum TxnPriorityRequirement {
kLowerPriorityRange,
kHigherPriorityRange,
kHighestPriority
} TxnPriorityRequirement;

// API to read type information.
const YBCPgTypeEntity *YBCPgFindTypeEntity(int type_oid);
YBCPgDataType YBCPgGetType(const YBCPgTypeEntity *type_entity);
Expand Down
8 changes: 0 additions & 8 deletions src/yb/yql/pggate/ybc_pggate.cc
View file
Expand Up @@ -934,14 +934,6 @@ YBCStatus YBCPgResetTransactionReadPoint() {
return ToYBCStatus(pgapi->ResetTransactionReadPoint());
}

double YBCGetTransactionPriority() {
return pgapi->GetTransactionPriority();
}

TxnPriorityRequirement YBCGetTransactionPriorityType() {
return pgapi->GetTransactionPriorityType();
}

YBCStatus YBCPgRestartReadPoint() {
return ToYBCStatus(pgapi->RestartReadPoint());
}
Expand Down
2 changes: 0 additions & 2 deletions src/yb/yql/pggate/ybc_pggate.h
View file
Expand Up @@ -458,8 +458,6 @@ YBCStatus YBCPgExitSeparateDdlTxnMode();
void YBCPgClearSeparateDdlTxnMode();
YBCStatus YBCPgSetActiveSubTransaction(uint32_t id);
YBCStatus YBCPgRollbackSubTransaction(uint32_t id);
double YBCGetTransactionPriority();
TxnPriorityRequirement YBCGetTransactionPriorityType();

// System validation -------------------------------------------------------------------------------
// Validate placement information
Expand Down

0 comments on commit 7cc503b

Please sign in to comment.