Re: Delay locking partitions during query execution

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Delay locking partitions during query execution
Date: 2019-01-28 07:45:24
Message-ID: f55c6839-2670-0d88-74ce-588ded6a0689@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2019/01/28 10:26, David Rowley wrote:
> On Tue, 4 Dec 2018 at 00:42, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
>> Over here and along similar lines to the above, but this time I'd like
>> to take this even further and change things so we don't lock *any*
>> partitions during AcquireExecutorLocks() and instead just lock them
>> when we first access them with ExecGetRangeTableRelation(). This
>> improves the situation when many partitions get run-time pruned, as
>> we'll never bother locking those at all since we'll never call
>> ExecGetRangeTableRelation() on them. We'll of course still lock the
>> partitioned table so that plan invalidation works correctly.
>
> I started looking at this patch again and I do see a problem with it.
> Let me explain:
>
> Currently during LockRelationOid() when we obtain a lock on a relation
> we'll check for rel cache invalidation messages. This means that
> during AcquireExecutorLocks(), as called from CheckCachedPlan(), we
> could miss invalidation messages since we're no longer necessarily
> locking the partitions.
>
> I think this only creates problems for partition's reloptions being
> changed and cached plans possibly being not properly invalidated when
> that happens, but I might be wrong about that, but if I'm not, there
> still also might be more important reasons to ensure we invalidate the
> plan properly in the future.

It seems to me that plancache.c doesn't really need to perform
AcquireExecutorLocks()/LockRelationOid() to learn that a partition's
reloptions property has changed to discard a generic plan and build a new
one. AFAICT, PlanCacheRelCallback() takes care of resetting a cached plan
by observing that an invalidation message that it received either from
the same session or from another session belongs to one of the relations
in PlannedStmt.relationOids. That list must already contain all
partitions' OIDs.

Session 1:
prepare q as select count(*) from p;
explain (costs off) execute q(1);
QUERY PLAN
────────────────────────────────────────────────────
Finalize Aggregate
-> Gather
Workers Planned: 2
-> Partial Aggregate
-> Parallel Append
-> Parallel Seq Scan on p4
-> Parallel Seq Scan on p1
-> Parallel Seq Scan on p2
-> Parallel Seq Scan on p3
-> Parallel Seq Scan on p_def
(10 rows)

-- same session (p1 can no longer use parallel scan)
alter table p1 set (parallel_workers=0);
explain (costs off) execute q(1);
QUERY PLAN
────────────────────────────────────────────────────
Finalize Aggregate
-> Gather
Workers Planned: 2
-> Partial Aggregate
-> Parallel Append
-> Seq Scan on p1
-> Parallel Seq Scan on p4
-> Parallel Seq Scan on p2
-> Parallel Seq Scan on p3
-> Parallel Seq Scan on p_def
(10 rows)

-- another session
alter table p1 reset (parallel_workers);

-- back to session 1 (p1 can now use parallel scan)
explain (costs off) execute q(1);
QUERY PLAN
────────────────────────────────────────────────────
Finalize Aggregate
-> Gather
Workers Planned: 2
-> Partial Aggregate
-> Parallel Append
-> Parallel Seq Scan on p4
-> Parallel Seq Scan on p1
-> Parallel Seq Scan on p2
-> Parallel Seq Scan on p3
-> Parallel Seq Scan on p_def
(10 rows)

Thanks,
Amit

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2019-01-28 08:05:26 Re: pg_basebackup, walreceiver and wal_sender_timeout
Previous Message Thomas Munro 2019-01-28 07:26:29 Re: BUG #15548: Unaccent does not remove combining diacritical characters