Relaxing the sub-transaction restriction in parallel query

From: Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Relaxing the sub-transaction restriction in parallel query
Date: 2021-06-14 04:20:03
Message-ID: CAJ3gD9fR1gLdJO9nqR5ynTRdu9Li8UxWp81fqmH_s7kVQUqoFA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Currently we don't allow a sub-transaction to be spawned from inside a
parallel worker (and also from a leader who is in parallel mode). This
imposes a restriction that pl/pgsql functions that use an exception block
can't be marked parallel safe, even when the exception block is there only
to catch trivial errors such as divide-by-zero. I tried to look at the
implications of removing this sub-transaction restriction, and came up with
the attached WIP patch after considering the below points. I may have
missed other points, or may have assumed something wrong. So comments are
welcome.

- TBLOCK_PARALLEL_INPROGRESS

Now that there can be an in-progress sub-transaction in a parallel worker,
the sub-transaction states need to be accounted for. Rather than having new
transaction states such as TBLOCK_PARALLEL_SUBINPROGRESS, I removed the
existing TBLOCK_PARALLEL_INPROGRESS from the code. At a couple of places
is_parallel_worker is set if state is TBLOCK_PARALLEL_INPROGRESS. Instead,
for now, I have used (ParallelCurrentXids != NULL) to identify if it's a
worker in a valid transaction state. Maybe we can improve on this.
In EndTransactionBlock(), there is a fatal error thrown if it's a
parallel-worker in-progress. This seems to be a can't-have case. So I have
removed this check. Need to further think how we can retain this check.

- IsInParallelMode()

On HEAD, the parallel worker cannot have any sub-transactions, so
CurrentTransactionState always points to the TopTransactionStateData. And
when ParallelWorkerMain() calls EnterParallelMode(), from that point
onwards IsInParallelMode() always returns true. But with the patch,
CurrentTransactionState can point to some nest level down below, and in
that TransactionState, parallelModeLevel would be 0, so IsInParallelMode()
will return false in a sub-transaction, unless some other function happens
to explicitly call EnterParallelMode().

One option for making IsInParallelMode() always return true for worker is
to just check whether the worker is in a transaction (ParallelCurrentXids
!= NULL). Or else, check only the TopTransactionData->parallelModeLevel.
Still another option is for the new TransactionState to inherit
parallelModeLevel from it's parent. I chose this option. This avoids
additional conditions in IsInParallelMode() specifically for worker.

Does this inherit-parent-parallelModeLevel option affect the leader code ?
The functions calling EnterParallelMode() are : begin_parallel_vacuum,
_bt_begin_parallel, ParallelWorkerMain, CommitTransaction, ExecutePlan().
After entering Parallel mode, it does not look like a subtransaction will
be spawned at these places. If at all it does, on HEAD, the
IsInParallelMode() will return false, which does not sound right. For all
the child transactions, this function should return true. So w.r.t. this,
in fact inheriting parent's parallelModeLevel looks better.

Operations that are not allowed to run in worker would continue to be
disallowed in a worker sub-transaction as well. E.g. assigning new xid,
heap_insert, etc. These places already are using IsInParallelMode() which
takes care of guarding against such operations.

Just for archival ...
ExecutePlan() is called with use_parallel_mode=true when there was a gather
plan created, in which case it enters Parallel mode. From here, if the
backend happens to start a new subtransaction for some reason, it does
sound right for the Parallel mode to be true for this sub-transaction,
although I am not sure if there can be such a case.
In worker, as far as I understand, ExecutePlan() always gets called with
use_parallel_mode=false, because there is no gather plan in the worker. So
it does not enter Parallel mode. But because the worker is already in
parallel mode, it does not matter.

- List of ParallelContexts (pcxt_list) :
ParallelContext is created only in backends. So there are no implications
of the pcxt_list w.r.t. parallel workers spawning a subtransaction, because
pcxt_list will always be empty in workers.

- ParallelCurrentXids :

A parallel worker always maintains a global flat sorted list of xids which
represent all the xids that are considered as current xids (i.e. the ones
that are returned by TransactionIdIsCurrentTransactionId() in a leader). So
this global list should continue to work no matter what is the
sub-transaction nest level, since there won't be new xids created in the
worker.

- Savepoints :

Haven't considered savepoints. The restriction is retained for savepoints.

Thanks
-Amit Khandekar
Huawei Technologies

Attachment Content-Type Size
allowsubtransinworker_wip.patch application/x-patch 14.5 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Shinya11.Kato 2021-06-14 04:53:58 RE: [PATCH] expand the units that pg_size_pretty supports on output
Previous Message Dilip Kumar 2021-06-14 04:14:01 Re: Decoding speculative insert with toast leaks memory