serializable transaction: exclude constraint violation (backed by GIST index) instead of ssi conflict

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

serializable transaction: exclude constraint violation (backed by GIST index) instead of ssi conflict

Peter Billen-2
Hi all,

I understood that v11 includes predicate locking for gist indexes, as per https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=3ad55863e9392bff73377911ebbf9760027ed405.

I tried this in combination with an exclude constraint as following:

drop table if exists t;
create table t(period tsrange);
alter table t add constraint bla exclude using gist(period with &&);
-- t1
begin transaction isolation level serializable;
select * from t where period && tsrange(now()::timestamp, now()::timestamp + interval '1 hour');
insert into t(period) values(tsrange(now()::timestamp, now()::timestamp + interval '1 hour'));
-- t2
begin transaction isolation level serializable;
select * from t where period && tsrange(now()::timestamp, now()::timestamp + interval '1 hour');
insert into t(period) values(tsrange(now()::timestamp, now()::timestamp + interval '1 hour'));
-- t1
commit;
-- t2
ERROR:  conflicting key value violates exclusion constraint "bla"
DETAIL:  Key (period)=(["2019-04-10 20:59:20.6265","2019-04-10 21:59:20.6265")) conflicts with existing key (period)=(["2019-04-10 20:59:13.332622","2019-04-10 21:59:13.332622")).

I kinda expected/hoped that transaction t2 would get aborted by a serialization error, and not an exclude constraint violation. This makes the application session bound to transaction t2 failing, as only serialization errors are retried.

We introduced the same kind of improvement/fix for btree indexes earlier, see https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=fcff8a575198478023ada8a48e13b50f70054766. Should this also be applied for (exclude) constraints backed by a gist index (as gist indexes now support predicate locking), or am I creating incorrect assumptions something here?

Thanks.
Reply | Threaded
Open this post in threaded view
|

Re: serializable transaction: exclude constraint violation (backed by GIST index) instead of ssi conflict

Thomas Munro-5
On Thu, Apr 11, 2019 at 9:43 AM Peter Billen <[hidden email]> wrote:

> I understood that v11 includes predicate locking for gist indexes, as per https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=3ad55863e9392bff73377911ebbf9760027ed405.
>
> I tried this in combination with an exclude constraint as following:
>
> drop table if exists t;
> create table t(period tsrange);
> alter table t add constraint bla exclude using gist(period with &&);
> -- t1
> begin transaction isolation level serializable;
> select * from t where period && tsrange(now()::timestamp, now()::timestamp + interval '1 hour');
> insert into t(period) values(tsrange(now()::timestamp, now()::timestamp + interval '1 hour'));
> -- t2
> begin transaction isolation level serializable;
> select * from t where period && tsrange(now()::timestamp, now()::timestamp + interval '1 hour');
> insert into t(period) values(tsrange(now()::timestamp, now()::timestamp + interval '1 hour'));
> -- t1
> commit;
> -- t2
> ERROR:  conflicting key value violates exclusion constraint "bla"
> DETAIL:  Key (period)=(["2019-04-10 20:59:20.6265","2019-04-10 21:59:20.6265")) conflicts with existing key (period)=(["2019-04-10 20:59:13.332622","2019-04-10 21:59:13.332622")).
>
> I kinda expected/hoped that transaction t2 would get aborted by a serialization error, and not an exclude constraint violation. This makes the application session bound to transaction t2 failing, as only serialization errors are retried.
>
> We introduced the same kind of improvement/fix for btree indexes earlier, see https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=fcff8a575198478023ada8a48e13b50f70054766. Should this also be applied for (exclude) constraints backed by a gist index (as gist indexes now support predicate locking), or am I creating incorrect assumptions something here?

Hi Peter,

Yeah, I agree, the behaviour you are expecting is desirable and we
should figure out how to do that.  The basic trick for btree unique
constraints was to figure out where the index *would* have written, to
give the SSI machinery a chance to object to that before raising the
UCV.  I wonder if we can use the same technique here... at first
glance, check_exclusion_or_unique_constraint() is raising the error,
but is not index AM specific code, and it is somewhat removed from the
GIST code that would do the equivalent
CheckForSerializableConflictIn() call.  I haven't looked into it
properly, but that certainly complicates matters somewhat...  Perhaps
the index AM would actually need a new entrypoint that could be called
before the error is raised, or perhaps there is an easier way.

--
Thomas Munro
https://enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: serializable transaction: exclude constraint violation (backed by GIST index) instead of ssi conflict

Thomas Munro-5
On Thu, Apr 11, 2019 at 10:54 AM Thomas Munro <[hidden email]> wrote:
> On Thu, Apr 11, 2019 at 9:43 AM Peter Billen <[hidden email]> wrote:
> > I kinda expected/hoped that transaction t2 would get aborted by a serialization error, and not an exclude constraint violation. This makes the application session bound to transaction t2 failing, as only serialization errors are retried.

> Yeah, I agree, the behaviour you are expecting is desirable and we
> should figure out how to do that.  The basic trick for btree unique
> constraints was to figure out where the index *would* have written, to
> give the SSI machinery a chance to object to that before raising the
> UCV.  I wonder if we can use the same technique here... at first
> glance, check_exclusion_or_unique_constraint() is raising the error,
> but is not index AM specific code, and it is somewhat removed from the
> GIST code that would do the equivalent
> CheckForSerializableConflictIn() call.  I haven't looked into it
> properly, but that certainly complicates matters somewhat...  Perhaps
> the index AM would actually need a new entrypoint that could be called
> before the error is raised, or perhaps there is an easier way.

Adding Kevin (architect of SSI and reviewer/committer of my UCV
interception patch) and Shubham (author of GIST SSI support) to the CC
list in case they have thoughts on this.

--
Thomas Munro
https://enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: serializable transaction: exclude constraint violation (backed by GIST index) instead of ssi conflict

Peter Billen-2


On Thu, Apr 11, 2019 at 1:14 AM Thomas Munro <[hidden email]> wrote:
On Thu, Apr 11, 2019 at 10:54 AM Thomas Munro <[hidden email]> wrote:
> On Thu, Apr 11, 2019 at 9:43 AM Peter Billen <[hidden email]> wrote:
> > I kinda expected/hoped that transaction t2 would get aborted by a serialization error, and not an exclude constraint violation. This makes the application session bound to transaction t2 failing, as only serialization errors are retried.

> Yeah, I agree, the behaviour you are expecting is desirable and we
> should figure out how to do that.  The basic trick for btree unique
> constraints was to figure out where the index *would* have written, to
> give the SSI machinery a chance to object to that before raising the
> UCV.  I wonder if we can use the same technique here... at first
> glance, check_exclusion_or_unique_constraint() is raising the error,
> but is not index AM specific code, and it is somewhat removed from the
> GIST code that would do the equivalent
> CheckForSerializableConflictIn() call.  I haven't looked into it
> properly, but that certainly complicates matters somewhat...  Perhaps
> the index AM would actually need a new entrypoint that could be called
> before the error is raised, or perhaps there is an easier way.

Adding Kevin (architect of SSI and reviewer/committer of my UCV
interception patch) and Shubham (author of GIST SSI support) to the CC
list in case they have thoughts on this.

Thanks Thomas, appreciated!

I was fiddling some more, and I am experiencing the same behavior with an exclude constraint backed by a btree index. I tried as following:

drop table if exists t;
create table t(i int);
alter table t add constraint bla exclude using btree(i with =);

-- t1
begin transaction isolation level serializable;
select * from t where i = 1;
insert into t(i) values(1);

-- t2
begin transaction isolation level serializable;
select * from t where i = 1;
insert into t(i) values(1);

-- t1
commit;

-- t2
ERROR:  conflicting key value violates exclusion constraint "bla"
DETAIL:  Key (i)=(1) conflicts with existing key (i)=(1).

Looking back, I now believe that https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=fcff8a575198478023ada8a48e13b50f70054766 was intended only for *unique* constraints, and not for *exclude* constraints as well. This is not explicitly mentioned in the commit message, though only tests for unique constraints are added in that commit.

I believe we are after multiple issues/improvements:

2. Fully support gist & constraints in serializable transactions. I did not yet test a unique constraint backed by a gist constraint, which is also interesting to test I assume. This test would tell us if there currently is a status quo between btree and gist indexes.

Thanks.
Reply | Threaded
Open this post in threaded view
|

Re: serializable transaction: exclude constraint violation (backed by GIST index) instead of ssi conflict

Peter Billen-2
On Thu, Apr 11, 2019 at 6:12 PM Peter Billen <[hidden email]> wrote:

I believe we are after multiple issues/improvements:

2. Fully support gist & constraints in serializable transactions. I did not yet test a unique constraint backed by a gist constraint, which is also interesting to test I assume. This test would tell us if there currently is a status quo between btree and gist indexes.

Regarding the remark in (2), I forgot that a unique constraint cannot be backed by a gist index, so forget the test I mentioned.
Reply | Threaded
Open this post in threaded view
|

Re: serializable transaction: exclude constraint violation (backed by GIST index) instead of ssi conflict

Thomas Munro-5
On Fri, Apr 12, 2019 at 6:01 AM Peter Billen <[hidden email]> wrote:
> On Thu, Apr 11, 2019 at 6:12 PM Peter Billen <[hidden email]> wrote:
>> I believe we are after multiple issues/improvements:
>>
>> 1. Could we extend https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=fcff8a575198478023ada8a48e13b50f70054766 by adding support for exclude constraints?
>> 2. Fully support gist & constraints in serializable transactions. I did not yet test a unique constraint backed by a gist constraint, which is also interesting to test I assume. This test would tell us if there currently is a status quo between btree and gist indexes.
>
> Regarding the remark in (2), I forgot that a unique constraint cannot be backed by a gist index, so forget the test I mentioned.

Yeah, well we can't directly extend the existing work because unique
constraints are *entirely* handled inside the btree code (in fact no
other index types even support unique constraints, yet).  This
exclusion constraints stuff is handled differently: the error handling
you're seeing is raised by generic code in
src/backend/executor/execIndexing.c , but the code that knows how to
actually perform the necessary SSI checks is index-specific, in this
case in gist.c.  To do the moral equivalent of the UCV change, we'll
need to get these two bits of code to communicate across the "index
AM" boundary (the way that index implementations such as GIST are
plugged into Postgres).  The question is how.

One (bad) idea is that we could actually perform the (illegal)
aminsert just before we raise that error!  We know we're going to roll
back anyway, because that's either going to fail when gist.c calls
CheckForSerializableConflictIn(), or if not, when we raise
"conflicting key value violates exclusion constraint ...".  That's a
bit messy though, because it modifies the index unnecessarily and
possibly breaks important invariants.  An improved version of that
idea is to add a new optional index AM interface "amcheckinsert()"
that shares whatever code it needs to share to do all the work that
insert would do except the actual modification.  That way,
check_exclusion_or_unique_constraint() would give every index AM a
chance to raise an SSI error if it wants to.  This seems like it
should work, but I don't want to propose messing around with the index
AM interface lightly.  It wouldn't usually get called, just in the
error path.

Anyone got a better idea?

--
Thomas Munro
https://enterprisedb.com