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] DROP SEQUENCE is not atomic #16309

Closed
jasonyb opened this issue Mar 4, 2023 · 0 comments
Closed

[YSQL] DROP SEQUENCE is not atomic #16309

jasonyb opened this issue Mar 4, 2023 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage
Projects

Comments

@jasonyb
Copy link
Contributor

jasonyb commented Mar 4, 2023

Jira Link: DB-5734

Description

Some objects are represented in both PG and YB (also colloquially referred to as "DocDB"). For example, tables: CREATE TABLE will first create the DocDB table then commit the pg metadata, and DROP TABLE will first remove the pg metadata then remove the DocDB table.

Sequences similarly have a PG representation and DocDB representation. However, it appears they have been neglected on the atomicity improvements that have been done on other objects like tables. So it may be possible for the sequence to exist in PG but not in DocDB or vice versa.

Here's an easy repro showing how DROP SEQUENCE may drop the sequence in DocDB but not in PG in case of a crash between the two internal steps:

Apply patch on master commit c92ad22:

diff --git i/src/postgres/src/backend/commands/sequence.c w/src/postgres/src/backend/commands/sequence.c
index d143502879..3cad67a9fd 100644
--- i/src/postgres/src/backend/commands/sequence.c
+++ w/src/postgres/src/backend/commands/sequence.c
@@ -624,6 +624,8 @@ DeleteSequenceTuple(Oid relid)
         HandleYBStatus(YBCDeleteSequenceTuple(MyDatabaseId, relid));
     }
 
+    elog(ERROR, "crash on drop");
+
     CatalogTupleDelete(rel, tuple);
 
     ReleaseSysCache(tuple);
bin/yb-ctl create --tserver_flags "ysql_sequence_cache_minval=0"
bin/ysqlsh
CREATE SEQUENCE s;
SELECT * FROM pg_sequence;
 seqrelid | seqtypid | seqstart | seqincrement |       seqmax        | seqmin | seqcache | seqcycle 
----------+----------+----------+--------------+---------------------+--------+----------+----------
    16384 |       20 |        1 |            1 | 9223372036854775807 |      1 |        1 | f
(1 row)
DROP SEQUENCE s;
ERROR:  crash on drop
-- reconnnect: this is optional but more closely resembles a crash
\c
SELECT nextval(16384);
ERROR:  Unable to find relation for sequence

cc: @deeps1991, @mislam-yb

Note that we can have issues with ALTER SEQUENCE RESET due to lack of atomicity. CREATE SEQUENCE can also possibly leave behind garbage rows in DocDB. However these issues are separately tracked in #19011 where we will convert the sequences table backed by DocDB into a transactional table that can be updated atomically with the PG catalog. This ticket merely tracks the fix for the failures incurred above due to DROP SEQUENCE not being atomic.

@jasonyb jasonyb added kind/bug This issue is a bug area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Mar 4, 2023
@jasonyb jasonyb added this to Backlog in YSQL via automation Mar 4, 2023
@yugabyte-ci yugabyte-ci added the priority/medium Medium priority issue label Mar 4, 2023
@deeps1991 deeps1991 changed the title [YSQL] Sequence operations aren't atomic [YSQL] DROP SEQUENCE is not atomic Sep 11, 2023
@deeps1991 deeps1991 self-assigned this Sep 11, 2023
deeps1991 added a commit that referenced this issue Sep 22, 2023
Summary:
Currently the sequences table hosted by the TServers is not transactional.
This means that DDL operations on sequences are not atomic - i.e. they
can reflect on the PG catalog but not on the DocDB sequences table.

Specifically for DROP SEQUENCES, the following scenario is possible:
```
CREATE SEQUENCE s;
SELECT * FROM pg_sequence;

 seqrelid | seqtypid | seqstart | seqincrement |       seqmax        | seqmin | seqcache | seqcycle
----------+----------+----------+--------------+---------------------+--------+----------+----------
    16384 |       20 |        1 |            1 | 9223372036854775807 |      1 |        1 | f
(1 row)

SET yb_test_fail_next_ddl =true;
DROP SEQUENCE s;
ERROR:  Failed DDL operation as requested

SELECT nextval(16384);
ERROR:  Unable to find relation for sequence
```

The above error is because of the following sequence of events:
1. YSQL backend starts up a transaction to update the pg_sequence table
    (among other catalog tables)
2. We then send a request to DocDB to delete the row corresponding to the
    sequence being dropped from the sequences table.
3. The error injection forces the transaction to fail causing the PG catalog
    changes to rollback, however we have already committed the removal of
    the row from the DocDB sequences table
4. Subsequent queries trying to access the sequences table will fail. DocDB will
    fail the operation with "Unable to find relation for sequence"

This patch has a partial fix for this issue. We now schedule the deletion
of the tuple pertaining to the sequence being deleted to be executed after
commit. This uses the infrastructure introduced in 3253827
to postpone deletion post commit.

The complete fix for this issue and other DDLs on sequences is tracked in #19011

Note: We cannot use the DDL atomicity infrastructure introduced in
6e604ba
as the sequences table is hosted on the TServers whereas the DDL
Atomicity infra is entirely in the YB-Master
Jira: DB-5734

Test Plan: ./yb_build.sh  --cxx-test pg_libpq-test --gtest_filter "*DropSequenceTest*"

Reviewers: myang

Reviewed By: myang

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D28324
YSQL automation moved this from Backlog to Done Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage
Projects
YSQL
  
Done
Development

No branches or pull requests

3 participants