Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[YSQL] duplicate key value violation on identity key column #22935

Closed
1 task done
markpyb opened this issue Jun 20, 2024 · 2 comments
Closed
1 task done

[YSQL] duplicate key value violation on identity key column #22935

markpyb opened this issue Jun 20, 2024 · 2 comments

Comments

@markpyb
Copy link

markpyb commented Jun 20, 2024

Jira Link: DB-11851

Description

When dropping a database with a sequence in it with ysql_sequence_cache_method=server, it is possible that the sequence cache is not invalidated, when the database is re-created with the application object (with a sequence) it leads to a situation where subsequent inserts can error with duplicate key value violation, on the identity key column

this seems present in 2024.1 and also 20.4

I observed it with a more complex test case , to observe the duplicate key value violation

create an app database with a identity key table and a secondary index on it, using server cache mode
run an pgbench with 120 parallel, each sqlfile is insert returning id, update where id
drop app database
re-create app database with the same ddl (apart from im slightly modifying the index each time for performance testing)
sometimes  after like 20 seconds
pgbench: error: client 16 script 0 aborted in command 54 query 0: ERROR:  duplicate key value violates unique constraint ""
pgbench: error: client 100 script 0 aborted in command 54 query 0: ERROR:  duplicate key value violates unique constraint ""
pgbench: error: client 50 script 0 aborted in command 54 query 0: ERROR:  duplicate key value violates unique constraint ""
pgbench: error: client 43 script 0 aborted in command 54 query 0: ERROR:  duplicate key value violates unique constraint ""

but a much simpler repro is possible to observe the behaviour when dropping the database

yugabyted destroy
yugabyted start --tserver_flags=ysql_sequence_cache_method=server
until postgres/bin/pg_isready -h $(hostname) ; do sleep 1 ; done | uniq
ysqlsh -h $(hostname)
\c yugabyte
create database d;
\c d
create sequence s;
select nextval('s');
select nextval('s');
\c yugabyte
drop database d;
create database d;
\c d
create sequence s;
select nextval('s');
yugabyte=# create database d;
CREATE DATABASE
yugabyte=# \c d
You are now connected to database "d" as user "yugabyte".
d=# create sequence s;
CREATE SEQUENCE
d=# select nextval('s');
 nextval
---------
       1
(1 row)

d=# select nextval('s');
 nextval
---------
       2
(1 row)

d=# \c yugabyte
You are now connected to database "yugabyte" as user "yugabyte".
yugabyte=# drop database d;
DROP DATABASE
yugabyte=# create database d;
CREATE DATABASE
yugabyte=# \c d
You are now connected to database "d" as user "yugabyte".
d=# create sequence s;
CREATE SEQUENCE
d=# select nextval('s');
 nextval
---------
       3
(1 row)

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@markpyb markpyb added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Jun 20, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jun 20, 2024
@andrei-mart
Copy link
Contributor

It turns out to be a server side cache id conflict.
Server uses sequence Oid as a cache id, it is unique in the database, but not in the cluster.
But cache entry is for whole node, hence other clients connected to the same node may end up sharing same cache entry, regardless the database they are connected to, if they happen to have same Oid in their respective databases.
Simplest repro, one session connects to freshly created database:

seqtest1=# create sequence foo;
CREATE SEQUENCE
seqtest1=# select oid, relname from pg_class where relkind = 'S';
  oid  | relname 
-------+---------
 16384 | foo
(1 row)

seqtest1=# select nextval('foo');
 nextval 
---------
       1
(1 row)

seqtest1=# select nextval('foo');
 nextval 
---------
       2
(1 row)

seqtest1=# select nextval('foo');
 nextval 
---------
       3
(1 row)

other session, connected to the same node, but other database creates brand new sequence, but it looks used:

seqtest2=# create sequence bar;
CREATE SEQUENCE
seqtest2=# select oid, relname from pg_class where relkind = 'S';
  oid  | relname 
-------+---------
 16384 | bar
(1 row)

seqtest2=# select nextval('bar');
 nextval 
---------
       4
(1 row)

The problem affects clusters with gflag ysql_sequence_cache_method=server.
By default ysql_sequence_cache_method=connection.

@andrei-mart
Copy link
Contributor

Related to that, sequence cache may have stale entries from dropped sequences and dropped databases, and they may conflict with active sessions.

@sushantrmishra sushantrmishra removed the status/awaiting-triage Issue awaiting triage label Jun 25, 2024
andrei-mart added a commit that referenced this issue Jun 27, 2024
Summary:
Since the tserver's sequence cache was introduced, the entry key was
the sequence's oid. However, it is not unique, each database has its
own oid generator, and each new database starts its oids from 16384,
hence high probability of collision in the sequence cache, which is
one instance per node and shared between all the databases.

Solution is to use both database oid and sequence oid as the entry key.
Database oid is unique between all databases within the cluster, and
the oids of the sequences are unique within the database.

Initially the bug was discovered when a database was dropped and
recreated, because the cache entries are not invalidated when sequence
is dropped. We may want to add invalidation logic, however, the problem
isn't critical with composite key: oids of dropped databases and other
objects are not recycled.
Jira: DB-11851

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgSequences#testMultiDbIsolation
./yb_build.sh --java-test org.yb.pgsql.TestPgSequencesWithServerCache#testMultiDbIsolation

Reviewers: jason

Reviewed By: jason

Subscribers: ybase, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36182
andrei-mart added a commit that referenced this issue Jun 27, 2024
…cache entry key

Summary:
Original commit: 214d44a / D36182
Since the tserver's sequence cache was introduced, the entry key was
the sequence's oid. However, it is not unique, each database has its
own oid generator, and each new database starts its oids from 16384,
hence high probability of collision in the sequence cache, which is
one instance per node and shared between all the databases.

Solution is to use both database oid and sequence oid as the entry key.
Database oid is unique between all databases within the cluster, and
the oids of the sequences are unique within the database.

Initially the bug was discovered when a database was dropped and
recreated, because the cache entries are not invalidated when sequence
is dropped. We may want to add invalidation logic, however, the problem
isn't critical with composite key: oids of dropped databases and other
objects are not recycled.
Jira: DB-11851

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgSequences#testMultiDbIsolation
./yb_build.sh --java-test org.yb.pgsql.TestPgSequencesWithServerCache#testMultiDbIsolation

Reviewers: jason

Reviewed By: jason

Subscribers: yql, ybase

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36214
jasonyb pushed a commit that referenced this issue Jun 28, 2024
Summary:
 9c637e2 [PLAT-14429]: Modify Troubleshooting Platform registration workflow in YBA
 0a1406d [PLAT-14098]: Updating yb.runtime_conf_ui.tag_filter with appropriate tags value does not display the flags accordingly
 70a87f9 [PLAT-13605][PLAT-13609]: Edit Volume controls and storage type in FULL_MOVE but not in case of UPDATE
 26fbfe0 [PLAT-14515][UI] Clicking preview doesn't show the correct info and clears up the data provided while setting up the ysql_ident or ysql_hba multiline flags.- [PLAT-14514] [PLAT-14513]
 a07946b [#18233] Initial commit for yugabyted unit test framework.
 b2e8ee7 [#22842] docdb: Improve usability of stack trace tracking endpoints
 508f26e [docs] Added RN 2.20.2.3-b2 (#23042)
 214d44a [#22935] YSQL: Use db oid in the tserver's sequence cache entry key
 c47b2d9 [#22802] YSQL: Avoid renaming DocDb tables during legacy rewrite
 Excluded: 7c8343d [#22874] YSQL: Fix cascaded drops on columns
 58c8d4e [#23046] xCluster: Remove ns_replication from the code
 a70681d [#22816] YSQL: Bug fixes for replication connections in ysql connection manager
 b239e07 Doc upgrade versions (#22988)
 Excluded: ec76062 [#16408] YSQL: Split TestPgRegressIndex.testPgRegressIndex

Test Plan: Jenkins: rebase: pg15-cherrypicks

Reviewers: jason, tfoucher

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36247
andrei-mart added a commit that referenced this issue Jul 1, 2024
…che entry key

Summary:
Original commit: 214d44a / D36182
Since the tserver's sequence cache was introduced, the entry key was
the sequence's oid. However, it is not unique, each database has its
own oid generator, and each new database starts its oids from 16384,
hence high probability of collision in the sequence cache, which is
one instance per node and shared between all the databases.

Solution is to use both database oid and sequence oid as the entry key.
Database oid is unique between all databases within the cluster, and
the oids of the sequences are unique within the database.

Initially the bug was discovered when a database was dropped and
recreated, because the cache entries are not invalidated when sequence
is dropped. We may want to add invalidation logic, however, the problem
isn't critical with composite key: oids of dropped databases and other
objects are not recycled.
Jira: DB-11851

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgSequences#testMultiDbIsolation
./yb_build.sh --java-test org.yb.pgsql.TestPgSequencesWithServerCache#testMultiDbIsolation

Reviewers: jason

Reviewed By: jason

Subscribers: ybase, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36215
andrei-mart added a commit that referenced this issue Jul 1, 2024
…che entry key

Summary:
Original commit: 214d44a / D36182
Since the tserver's sequence cache was introduced, the entry key was
the sequence's oid. However, it is not unique, each database has its
own oid generator, and each new database starts its oids from 16384,
hence high probability of collision in the sequence cache, which is
one instance per node and shared between all the databases.

Solution is to use both database oid and sequence oid as the entry key.
Database oid is unique between all databases within the cluster, and
the oids of the sequences are unique within the database.

Initially the bug was discovered when a database was dropped and
recreated, because the cache entries are not invalidated when sequence
is dropped. We may want to add invalidation logic, however, the problem
isn't critical with composite key: oids of dropped databases and other
objects are not recycled.
Jira: DB-11851

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgSequences#testMultiDbIsolation
./yb_build.sh --java-test org.yb.pgsql.TestPgSequencesWithServerCache#testMultiDbIsolation

Reviewers: jason

Reviewed By: jason

Subscribers: yql, ybase

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36216
andrei-mart added a commit that referenced this issue Jul 1, 2024
…cache entry key

Summary:
Original commit: 214d44a / D36182
Since the tserver's sequence cache was introduced, the entry key was
the sequence's oid. However, it is not unique, each database has its
own oid generator, and each new database starts its oids from 16384,
hence high probability of collision in the sequence cache, which is
one instance per node and shared between all the databases.

Solution is to use both database oid and sequence oid as the entry key.
Database oid is unique between all databases within the cluster, and
the oids of the sequences are unique within the database.

Initially the bug was discovered when a database was dropped and
recreated, because the cache entries are not invalidated when sequence
is dropped. We may want to add invalidation logic, however, the problem
isn't critical with composite key: oids of dropped databases and other
objects are not recycled.
Jira: DB-11851

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgSequences#testMultiDbIsolation
./yb_build.sh --java-test org.yb.pgsql.TestPgSequencesWithServerCache#testMultiDbIsolation

Reviewers: jason

Reviewed By: jason

Subscribers: yql, ybase

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36227
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants