Обсуждение: serializable transaction: exclude constraint violation (backed byGIST index) instead of ssi conflict
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.
On Thu, Apr 11, 2019 at 9:43 AM Peter Billen <peter.billen@gmail.com> 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-1020: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 alsobe applied for (exclude) constraints backed by a gist index (as gist indexes now support predicate locking), or am Icreating 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
On Thu, Apr 11, 2019 at 10:54 AM Thomas Munro <thomas.munro@gmail.com> wrote: > On Thu, Apr 11, 2019 at 9:43 AM Peter Billen <peter.billen@gmail.com> wrote: > > I kinda expected/hoped that transaction t2 would get aborted by a serialization error, and not an exclude constraintviolation. This makes the application session bound to transaction t2 failing, as only serialization errors areretried. > 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
On Thu, Apr 11, 2019 at 1:14 AM Thomas Munro <thomas.munro@gmail.com> wrote:
On Thu, Apr 11, 2019 at 10:54 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Thu, Apr 11, 2019 at 9:43 AM Peter Billen <peter.billen@gmail.com> 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:
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.
Thanks.
On Thu, Apr 11, 2019 at 6:12 PM Peter Billen <peter.billen@gmail.com> 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.
On Fri, Apr 12, 2019 at 6:01 AM Peter Billen <peter.billen@gmail.com> wrote: > On Thu, Apr 11, 2019 at 6:12 PM Peter Billen <peter.billen@gmail.com> wrote: >> I believe we are after multiple issues/improvements: >> >> 1. Could we extend https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=fcff8a575198478023ada8a48e13b50f70054766by adding supportfor exclude constraints? >> 2. Fully support gist & constraints in serializable transactions. I did not yet test a unique constraint backed by a gistconstraint, which is also interesting to test I assume. This test would tell us if there currently is a status quo betweenbtree 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