Re: ATTACH/DETACH PARTITION CONCURRENTLY

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Sergei Kornilov <sk(at)zsrv(dot)org>, Amit Langote <langote_amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ATTACH/DETACH PARTITION CONCURRENTLY
Date: 2019-01-29 16:52:25
Message-ID: CA+Tgmobq8atpwshzvqV_eriS4TX==+k1Hx-KWxCjj5ROLDz8KQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 29, 2019 at 12:29 AM Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> But I kind of wonder whether we're really gaining as much as you think
>> by trying to support concurrent DETACH in the first place. If there
>> are queries running against the table, there's probably at least
>> AccessShareLock on the partition itself, not just the parent. And
>> that means that reducing the necessary lock on the parent from
>> AccessExclusiveLock to something less doesn't really help that much,
>> because we're still going to block trying to acquire
>> AccessExclusiveLock on the child. Now you might say: OK, well, just
>> reduce that lock level, too.
>
> The whole point of CONCURRENT detach is that you're not removing it whilst people are still using it, you're just marking it for later disuse.

Well, I don't think that's the way any patch so far proposed actually works.

>> But that seems to me to be opening a giant can of worms which we are
>> unlikely to get resolved in time for this release. The worst of those
>> problem is that if you also reduce the lock level on the partition
>> when attaching it, then you are adding a constraint while somebody
>> might at the exact same time be inserting a tuple that violates that
>> constraint.
>
> Spurious.
>
> This would only be true if we were adding a constraint that affected existing partitions.
>
> The constraint being added affects the newly added partition, not existing ones.

I agree that it affects the newly added partition, not existing ones.
But if you don't hold an AccessExclusiveLock on that partition while
you are adding that constraint to it, then somebody could be
concurrently inserting a tuple that violates that constraint. This
would be an INSERT targeting the partition directly, not somebody
operating on the partitioning hierarchy to which it is being attached.

>> Those two things have to be synchronized somehow. You
>> could avoid that by reducing the lock level on the partition when
>> detaching and not when attaching. But even then, detaching a
>> partition can involve performing a whole bunch of operations for which
>> we currently require AccessExclusiveLock. AlterTableGetLockLevel says:
>>
>> /*
>> * Removing constraints can affect SELECTs that have been
>> * optimised assuming the constraint holds true.
>> */
>> case AT_DropConstraint: /* as DROP INDEX */
>> case AT_DropNotNull: /* may change some SQL plans */
>> cmd_lockmode = AccessExclusiveLock;
>> break;
>>
>> Dropping a partition implicitly involves dropping a constraint. We
>> could gamble that the above has no consequences that are really
>
> It's not a gamble if you know that the constraints being dropped constrain only the object being dropped.

That's not true, but I can't refute your argument any more than that
because you haven't made one.

> I've not read every argument on this thread, but many of the later points made here are spurious, by which I mean they sound like they could apply but in fact do not.

I think they do apply, and until somebody explains convincingly why
they don't, I'm going to keep thinking that they do. Telling me that
my points are wrong without making any kind of argument about why they
are wrong is not constructive. I've put a lot of energy into
analyzing this topic, both recently and in previous release cycles,
and I'm not inclined to just say "OK, well, Simon says I'm wrong, so
that's the end of it."

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andreas Karlsson 2019-01-29 17:00:29 Re: Covering GiST indexes
Previous Message Andres Freund 2019-01-29 16:51:37 Re: Rename nodes/relation.h => nodes/pathnodes.h ?