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

Commit e6a9637

Browse files
committed
Revert "Allow parallel workers to cope with a newly-created session user ID."
This reverts commit f5f30c2. 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 ca2eea3 commit e6a9637

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
@@ -1329,21 +1329,3 @@ SELECT 1 FROM tenk1_vw_sec
13291329
(9 rows)
13301330

13311331
rollback;
1332-
-- test that a newly-created session role propagates to workers.
1333-
begin;
1334-
create role regress_parallel_worker;
1335-
set session authorization regress_parallel_worker;
1336-
select current_setting('session_authorization');
1337-
current_setting
1338-
-------------------------
1339-
regress_parallel_worker
1340-
(1 row)
1341-
1342-
set debug_parallel_query = 1;
1343-
select current_setting('session_authorization');
1344-
current_setting
1345-
-------------------------
1346-
regress_parallel_worker
1347-
(1 row)
1348-
1349-
rollback;

src/test/regress/sql/select_parallel.sql

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

513513
rollback;
514-
515-
-- test that a newly-created session role propagates to workers.
516-
begin;
517-
create role regress_parallel_worker;
518-
set session authorization regress_parallel_worker;
519-
select current_setting('session_authorization');
520-
set debug_parallel_query = 1;
521-
select current_setting('session_authorization');
522-
rollback;

0 commit comments

Comments
 (0)