Skip to content

Commit

Permalink
[#23787] YSQL: Avoid executing conn mgr guc variables hooks for paral…
Browse files Browse the repository at this point in the history
…lel workers

Summary:
This diff avoids executing check/assign hooks of ysql connection manager specific guc variables while initialising the parallel background worker.

**Reason for crash**
The hooks for connection manager specific guc variables `yb_is_client_ysqlconnmgr`, `yb_use_tserver_key_auth`  were failing for parallel background workers. These guc variables are set by ysql connection manager while creating the physical connections and expects connections to be of unix socket type. Whereas for parallel background workers created by postmaster  it's not necessary nor these guc variables are applicable. Therefore avoiding the hooks.

This diff fixes the tests mentioned in test plan which used to fail due to same above reason. Tests are failing only with tsan build but not due to this diff/reason, it's been tracked here [[ https://yugabyte.atlassian.net/browse/DB-12641 | DB-12641 ]]
Jira: DB-12690

Test Plan: Jenkins: enable connection manager, test regex: .*PgParallelReadIsolation.*|.*PgTransparentRestarts.*

Reviewers: skumar, asrinivasan, stiwary, devansh.saxena, rbarigidad

Reviewed By: skumar, stiwary

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D37767
  • Loading branch information
Manav Kumar committed Sep 5, 2024
1 parent 4c6cf5a commit f24eb10
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 3 deletions.
3 changes: 3 additions & 0 deletions src/postgres/src/backend/access/transam/parallel.c
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,9 @@ ParallelWorkerMain(Datum main_arg)
/* Set flag to indicate that we're initializing a parallel worker. */
InitializingParallelWorker = true;

if (YBIsEnabledInPostgresEnvVar())
YbSetParallelWorker();

/* Establish signal handlers. */
pqsignal(SIGTERM, die);
BackgroundWorkerUnblockSignals();
Expand Down
6 changes: 6 additions & 0 deletions src/postgres/src/backend/utils/misc/guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -11595,6 +11595,12 @@ read_gucstate_binary(char **srcptr, char *srcend, void *dest, Size size)
*srcptr += size;
}

void YbSetParallelWorker()
{
yb_is_parallel_worker = true;
elog(LOG, "yb_is_parallel_worker has been set to true");
}

/*
* RestoreGUCState:
* Reads the GUC state at the specified address and updates the GUCs with the
Expand Down
8 changes: 7 additions & 1 deletion src/postgres/src/backend/utils/misc/pg_yb_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -4677,7 +4677,13 @@ bool
yb_use_tserver_key_auth_check_hook(bool *newval, void **extra, GucSource source)
{
/* Allow setting yb_use_tserver_key_auth to false */
if (!(*newval))
/*
* Parallel workers are created and maintained by postmaster. So physical connections
* can never be of parallel worker type, therefore it makes no sense to restore
* or even do check/assign hooks for ysql connection manager specific guc variables
* on parallel worker process.
*/
if (!(*newval) || yb_is_parallel_worker == true)
return true;

/*
Expand Down
16 changes: 14 additions & 2 deletions src/postgres/src/backend/utils/misc/yb_ysql_conn_mgr_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
#define YB_CREATE_SHMEM_FLAG 0666 | IPC_EXCL | IPC_CREAT

bool yb_is_client_ysqlconnmgr = false;
bool yb_is_parallel_worker = false;

enum SESSION_PARAMETER_UPDATE_RST
{
Expand Down Expand Up @@ -797,7 +798,13 @@ yb_is_client_ysqlconnmgr_check_hook(bool *newval, void **extra,
GucSource source)
{
/* Allow setting yb_is_client_ysqlconnmgr as false */
if (!(*newval))
/*
* Parallel workers are created and maintained by postmaster. So physical connections
* can never be of parallel worker type, therefore it makes no sense to restore
* or even do check/assign hooks for ysql connection manager specific guc variables
* on parallel worker process.
*/
if (!(*newval) || yb_is_parallel_worker == true)
return true;

/* Client needs to be connected on unix domain socket */
Expand All @@ -821,7 +828,12 @@ yb_is_client_ysqlconnmgr_assign_hook(bool newval, void *extras)
{
yb_is_client_ysqlconnmgr = newval;

if (yb_is_client_ysqlconnmgr == true)
/*
* Parallel workers are created and maintained by postmaster. So physical connections
* can never be of parallel worker type, therefore it makes no sense to perform any
* ysql connection manager specific operations on it.
*/
if (yb_is_client_ysqlconnmgr == true && !yb_is_parallel_worker)
send_oid_info('d', get_database_oid(MyProcPort->database_name, false));
}

Expand Down
2 changes: 2 additions & 0 deletions src/postgres/src/include/utils/guc.h
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,8 @@ extern void write_nondefault_variables(GucContext context);
extern void read_nondefault_variables(void);
#endif

extern void YbSetParallelWorker();

/* GUC serialization */
extern Size EstimateGUCStateSpace(void);
extern void SerializeGUCState(Size maxsize, char *start_address);
Expand Down
6 changes: 6 additions & 0 deletions src/postgres/src/include/yb_ysql_conn_mgr_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@
*/
extern bool yb_is_client_ysqlconnmgr;

/*
* `yb_is_parallel_worker` is used to identify that whether background worker is
* of parallel worker type.
*/
extern bool yb_is_parallel_worker;

/* TODO (janand): Write a function to read/change yb_logical_client_shmem_key */
extern int yb_logical_client_shmem_key;

Expand Down

0 comments on commit f24eb10

Please sign in to comment.