Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 630b81d

Browse files
committed
Revert "Allow parallel workers to cope with a newly-created session user ID."
This reverts commit 5887dd4. Some buildfarm animals are failing with "cannot change "client_encoding" during a parallel operation". It looks like assign_client_encoding is unhappy at being asked to roll back a client_encoding setting after a parallel worker encounters a failure. There must be more to it though: why didn't I see this during local testing? In any case, it's clear that moving the RestoreGUCState() call is not as side-effect-free as I thought. Given that the bug f5f30c2 intended to fix has gone unreported for years, it's not something that's urgent to fix; I'm not willing to risk messing with it further with only days to our next release wrap.
1 parent 5887dd4 commit 630b81d

File tree

4 files changed

+6
-43
lines changed

4 files changed

+6
-43
lines changed

src/backend/access/transam/parallel.c

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,6 +1410,10 @@ ParallelWorkerMain(Datum main_arg)
14101410
libraryspace = shm_toc_lookup(toc, PARALLEL_KEY_LIBRARY, false);
14111411
StartTransactionCommand();
14121412
RestoreLibraryState(libraryspace);
1413+
1414+
/* Restore GUC values from launching backend. */
1415+
gucspace = shm_toc_lookup(toc, PARALLEL_KEY_GUC, false);
1416+
RestoreGUCState(gucspace);
14131417
CommitTransactionCommand();
14141418

14151419
/* Crank up a transaction state appropriate to a parallel worker. */
@@ -1451,14 +1455,6 @@ ParallelWorkerMain(Datum main_arg)
14511455
*/
14521456
InvalidateSystemCaches();
14531457

1454-
/*
1455-
* Restore GUC values from launching backend. We can't do this earlier,
1456-
* because GUC check hooks that do catalog lookups need to see the same
1457-
* database state as the leader.
1458-
*/
1459-
gucspace = shm_toc_lookup(toc, PARALLEL_KEY_GUC, false);
1460-
RestoreGUCState(gucspace);
1461-
14621458
/*
14631459
* Restore current role id. Skip verifying whether session user is
14641460
* allowed to become this role and blindly restore the leader's state for

src/backend/commands/variable.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -577,16 +577,14 @@ check_transaction_read_only(bool *newval, void **extra, GucSource source)
577577
* We allow idempotent changes at any time, but otherwise this can only be
578578
* changed in a toplevel transaction that has not yet taken a snapshot.
579579
*
580-
* As in check_transaction_read_only, allow it if not inside a transaction,
581-
* or if restoring state in a parallel worker.
580+
* As in check_transaction_read_only, allow it if not inside a transaction.
582581
*/
583582
bool
584583
check_transaction_isolation(int *newval, void **extra, GucSource source)
585584
{
586585
int newXactIsoLevel = *newval;
587586

588-
if (newXactIsoLevel != XactIsoLevel &&
589-
IsTransactionState() && !InitializingParallelWorker)
587+
if (newXactIsoLevel != XactIsoLevel && IsTransactionState())
590588
{
591589
if (FirstSnapshotSet)
592590
{
@@ -621,10 +619,6 @@ check_transaction_isolation(int *newval, void **extra, GucSource source)
621619
bool
622620
check_transaction_deferrable(bool *newval, void **extra, GucSource source)
623621
{
624-
/* Just accept the value when restoring state in a parallel worker */
625-
if (InitializingParallelWorker)
626-
return true;
627-
628622
if (IsSubTransaction())
629623
{
630624
GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION);

src/test/regress/expected/select_parallel.out

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,21 +1294,3 @@ SELECT 1 FROM tenk1_vw_sec
12941294
(9 rows)
12951295

12961296
rollback;
1297-
-- test that a newly-created session role propagates to workers.
1298-
begin;
1299-
create role regress_parallel_worker;
1300-
set session authorization regress_parallel_worker;
1301-
select current_setting('session_authorization');
1302-
current_setting
1303-
-------------------------
1304-
regress_parallel_worker
1305-
(1 row)
1306-
1307-
set debug_parallel_query = 1;
1308-
select current_setting('session_authorization');
1309-
current_setting
1310-
-------------------------
1311-
regress_parallel_worker
1312-
(1 row)
1313-
1314-
rollback;

src/test/regress/sql/select_parallel.sql

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -496,12 +496,3 @@ SELECT 1 FROM tenk1_vw_sec
496496
WHERE (SELECT sum(f1) FROM int4_tbl WHERE f1 < unique1) < 100;
497497

498498
rollback;
499-
500-
-- test that a newly-created session role propagates to workers.
501-
begin;
502-
create role regress_parallel_worker;
503-
set session authorization regress_parallel_worker;
504-
select current_setting('session_authorization');
505-
set debug_parallel_query = 1;
506-
select current_setting('session_authorization');
507-
rollback;

0 commit comments

Comments
 (0)