pgsql: Don't lock partitions pruned by initial pruning - Mailing list pgsql-committers

From Amit Langote
Subject pgsql: Don't lock partitions pruned by initial pruning
Date
Msg-id E1tl1f3-000H54-0k@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Don't lock partitions pruned by initial pruning

Before executing a cached generic plan, AcquireExecutorLocks() in
plancache.c locks all relations in a plan's range table to ensure the
plan is safe for execution. However, this locks runtime-prunable
relations that will later be pruned during "initial" runtime pruning,
introducing unnecessary overhead.

This commit defers locking for such relations to executor startup and
ensures that if the CachedPlan is invalidated due to concurrent DDL
during this window, replanning is triggered. Deferring these locks
avoids unnecessary locking overhead for pruned partitions, resulting
in significant speedup, particularly when many partitions are pruned
during initial runtime pruning.

* Changes to locking when executing generic plans:

AcquireExecutorLocks() now locks only unprunable relations, that is,
those found in PlannedStmt.unprunableRelids (introduced in commit
cbc127917e), to avoid locking runtime-prunable partitions
unnecessarily.  The remaining locks are taken by
ExecDoInitialPruning(), which acquires them only for partitions that
survive pruning.

This deferral does not affect the locks required for permission
checking in InitPlan(), which takes place before initial pruning.
ExecCheckPermissions() now includes an Assert to verify that all
relations undergoing permission checks, none of which can be in the
set of runtime-prunable relations, are properly locked.

* Plan invalidation handling:

Deferring locks introduces a window where prunable relations may be
altered by concurrent DDL, invalidating the plan. A new function,
ExecutorStartCachedPlan(), wraps ExecutorStart() to detect and handle
invalidation caused by deferred locking. If invalidation occurs,
ExecutorStartCachedPlan() updates CachedPlan using the new
UpdateCachedPlan() function and retries execution with the updated
plan. To ensure all code paths that may be affected by this handle
invalidation properly, all callers of ExecutorStart that may execute a
PlannedStmt from a CachedPlan have been updated to use
ExecutorStartCachedPlan() instead.

UpdateCachedPlan() replaces stale plans in CachedPlan.stmt_list. A new
CachedPlan.stmt_context, created as a child of CachedPlan.context,
allows freeing old PlannedStmts while preserving the CachedPlan
structure and its statement list. This ensures that loops over
statements in upstream callers of ExecutorStartCachedPlan() remain
intact.

ExecutorStart() and ExecutorStart_hook implementations now return a
boolean value indicating whether plan initialization succeeded with a
valid PlanState tree in QueryDesc.planstate, or false otherwise, in
which case QueryDesc.planstate is NULL. Hook implementations are
required to call standard_ExecutorStart() at the beginning, and if it
returns false, they should do the same without proceeding.

* Testing:

To verify these changes, the delay_execution module tests scenarios
where cached plans become invalid due to changes in prunable relations
after deferred locks.

* Note to extension authors:

ExecutorStart_hook implementations must verify plan validity after
calling standard_ExecutorStart(), as explained earlier. For example:

    if (prev_ExecutorStart)
        plan_valid = prev_ExecutorStart(queryDesc, eflags);
    else
        plan_valid = standard_ExecutorStart(queryDesc, eflags);

    if (!plan_valid)
        return false;

    <extension-code>

    return true;

Extensions accessing child relations, especially prunable partitions,
via ExecGetRangeTableRelation() must now ensure their RT indexes are
present in es_unpruned_relids (introduced in commit cbc127917e), or
they will encounter an error. This is a strict requirement after this
change, as only relations in that set are locked.

The idea of deferring some locks to executor startup, allowing locks
for prunable partitions to be skipped, was first proposed by Tom Lane.

Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier versions)
Reviewed-by: David Rowley <dgrowleyml@gmail.com> (earlier versions)
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (earlier versions)
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/525392d5727f469e9a5882e1d728917a4be56147

Modified Files
--------------
contrib/auto_explain/auto_explain.c                |  16 +-
contrib/pg_stat_statements/pg_stat_statements.c    |  16 +-
src/backend/commands/copyto.c                      |   5 +-
src/backend/commands/createas.c                    |   5 +-
src/backend/commands/explain.c                     |  22 +-
src/backend/commands/extension.c                   |   4 +-
src/backend/commands/matview.c                     |   5 +-
src/backend/commands/portalcmds.c                  |   1 +
src/backend/commands/prepare.c                     |   9 +-
src/backend/commands/trigger.c                     |  15 ++
src/backend/executor/README                        |  35 ++-
src/backend/executor/execMain.c                    | 127 ++++++++++-
src/backend/executor/execParallel.c                |  12 +-
src/backend/executor/execPartition.c               |  38 +++-
src/backend/executor/execUtils.c                   |   8 +
src/backend/executor/functions.c                   |   4 +-
src/backend/executor/spi.c                         |  29 ++-
src/backend/tcop/postgres.c                        |   4 +-
src/backend/tcop/pquery.c                          |  51 ++++-
src/backend/utils/cache/plancache.c                | 197 +++++++++++++---
src/backend/utils/mmgr/portalmem.c                 |   4 +-
src/include/commands/explain.h                     |   6 +-
src/include/commands/trigger.h                     |   1 +
src/include/executor/execdesc.h                    |   2 +
src/include/executor/executor.h                    |  34 ++-
src/include/nodes/execnodes.h                      |   3 +
src/include/utils/plancache.h                      |  46 +++-
src/include/utils/portal.h                         |   4 +-
src/test/modules/delay_execution/Makefile          |   3 +-
src/test/modules/delay_execution/delay_execution.c |  66 +++++-
.../delay_execution/expected/cached-plan-inval.out | 250 +++++++++++++++++++++
src/test/modules/delay_execution/meson.build       |   1 +
.../delay_execution/specs/cached-plan-inval.spec   |  86 +++++++
33 files changed, 1014 insertions(+), 95 deletions(-)


pgsql-committers by date:

Previous
From: Amit Kapila
Date:
Subject: pgsql: Include schema/table publications even with exclude options in d
Next
From: Amit Kapila
Date:
Subject: pgsql: Improve errdetail message added by ac0e33136a.