[YSQL] Rare crash of TestPgRegressPgMiscIndependent#testPgRegressPgMiscIndependent #12185
Assignees
Labels
area/ysql
Yugabyte SQL (YSQL)
kind/bug
This issue is a bug
status/awaiting-triage
Issue awaiting triage
Projects
Comments
d-uspenskiy
added
kind/bug
d-uspenskiy
added a commit
that referenced
this issue
Apr 29, 2022
…ggate to postgres
Summary:
In postgres the `duplicate key` error message contains constraint name.
```
ERROR: duplicate key value violates unique constraint "t1_pkey"
```
Nowadays YSQL fetches constraint name in the `PgSession::PatchStatus` method by using the callback of postgres callback.
This strategy leads to undesired behavior because the `PgSession::PatchStatus` might be called from the stack after receiving response of flushed operations. And postgres callback might make a read from sys table (in case constraint name is not in a cache yet). And the read from sys table will flush the buffered operations once again before sending read RPC. As a result, patching status on receiving response of flushing operations might trigger flushing of the next buffer of operations. And second flushing might fail with another error (including another `duplicate key`). As a result initial error will be substituted with new one.
Here is the example scenario:
```
CREATE TABLE t1(k INT PRIMARY KEY);
CREATE TABLE t2(k INT PRIMARY KEY);
INSERT INTO t1 VALUES(1);
INSERT INTO t2 VALUES(1);
SET ysql_session_max_batch_size = 3;
DO $$
BEGIN
INSERT INTO t1 VALUES(3), (2), (1); -- the 'duplicate key' error is raised here because k = 1 already exists in t1
INSERT INTO t2 VALUES(1); -- the 'duplicate key' error is raised here because k = 1 already exists in t2, but vanilla postgres will not reach this line
INSERT INTO t1 VALUES(2);
END
$$;
ERROR: duplicate key value violates unique constraint "t2_pkey"
```
Instead of error related to `t1_pkey` YSQL shows error related to `t2_pkey`.
Another problem of resolving constraint name in the `PgSession::PatchStatus` method is that this message can be called from cleanup procedures (destructors) and it is not possible to make a read from this stack.
Example:
```
PgDocOp::~PgDocOp() {
// Wait for result in case request was sent.
// Operation can be part of transaction it is necessary to complete it before transaction commit.
if (response_.Valid()) {
WARN_NOT_OK(ResultToStatus(response_.Get()), "Operation completion failed"); <-- the PgSession::PatchStatus method will be called in case of error
}
}
```
This scenario is the root cause of the `org.yb.pgsql.TestPgRegressPgMiscIndependent#testPgRegressPgMiscIndependent` test failures.
Solution is to postpone constraint name resolving until the point where postgres uses error message (the `HandleYBStatusAtErrorLevel` function). For this purpose in the `PgSession::PatchStatus` method constraint's id is added to error status.
**Note:**
1. The `TestOperationBatching` unit test is removed. All its scenarios are re-implemented in newly created `PgOpBufferingTest` C++ unit test.
2. The `org.yb.pgsql.TestPgRegressPgMiscIndependent#testPgRegressPgMiscIndependent` unit test is fixed by current diff.
Test Plan:
New unit test was introduced
```
./yb_build.sh --gtest_filter PgOpBufferingTest.MultipleConstraintsConflict
```
Reviewers: nli, alex, pjain
Reviewed By: pjain
Subscribers: sergei, yql
Differential Revision: https://phabricator.dev.yugabyte.com/D16581
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Sometimes the TestPgRegressPgMiscIndependent#testPgRegressPgMiscIndependent may crash with the following stack
Possible reason is that ysql reads data from system catalog when processing the
duplicate key value violates unique constrainterror response to fetch constraints name.The text was updated successfully, but these errors were encountered: