Skip to content

Commit

Permalink
[#23843] YSQL: Do not retry schema mismatch errors that occur as part…
Browse files Browse the repository at this point in the history
… of a batched execution.

Summary:
### Motivation

Commit fa8a138 fixes an issue with internal retries of transaction conflicts in batched execution scenarios. Retries for schema mismatch errors take a different code path. Do not retry schema mismatch errors when in batched mode of execution.

Say, we retry 5th statement of the batch on a schema version mismatch error.

```
.addBatch("UPDATE t SET v = 2 WHERE k = 1")
.addBatch("UPDATE t SET v = 2 WHERE k = 2")
.addBatch("UPDATE t SET v = 2 WHERE k = 3")
.addBatch("UPDATE t SET v = 2 WHERE k = 4")
.addBatch("UPDATE t SET v = 2 WHERE k = 5") <-- retry here
...
.addBatch("UPDATE t SET v = 2 WHERE k = 10")

.executeBatch()
```

Only statements starting from the retried statement take effect. That is, internally,

```
START TXN
UPDATE ... k = 1
UPDATE ... k = 2
UPDATE ... k = 3
UPDATE ... k = 4
UPDATE ... k = 5
<--- schema mismatch error
<--- abort txn and retry
ROLLBACK TXN
START TXN
UPDATE ... k = 5 <--- retries starting from 5th statement because the previous statements are lost
...
UPDATE ... k = 10
COMMIT TXN
```

This is a transaction atomicity issue.

### Fix

Schema mismatch errors are not retried when the statement exists within an explicit transaction block. Similarly, avoid retrying transactions in batched execution mode.
Jira: DB-12746

Test Plan:
Jenkins

```
./yb_build.sh --java-test TestPgBatch#testTransparentRestartSchemaMismatch
```

**Context:** TestPgBatch.java executes a conflicting statement `UPDATE t SET v=1 WHERE k=...` to test retries due to transaction conflicts in batched execution mode.

Execute `ALTER TABLE t ALTER COLUMN v SET NOT NULL` to introduce a schema mismatch error instead of a transaction conflict. This triggers the retry path meant for schema mismatch errors. We verify that an exception is raised in batched execution mode.

Backport-through: 2.20

Reviewers: tfoucher, dmitry, smishra

Reviewed By: tfoucher, dmitry

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D37913
  • Loading branch information
pao214 committed Sep 12, 2024
1 parent b2d16eb commit dd60793
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
36 changes: 36 additions & 0 deletions java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgBatch.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.yb.AssertionWrappers.assertGreaterThan;
import static org.yb.AssertionWrappers.assertNotEquals;
import static org.yb.AssertionWrappers.assertTrue;
import static org.yb.AssertionWrappers.fail;

import com.yugabyte.util.PSQLException;
import java.sql.BatchUpdateException;
Expand Down Expand Up @@ -49,6 +50,8 @@ protected Map<String, String> getTServerFlags() {
flagMap.put("yb_enable_read_committed_isolation", "true");
// TODO: Remove this override when wait queues are enabled by default.
flagMap.put("enable_wait_queues", "true");
// Easier debugging.
flagMap.put("ysql_log_statement", "all");
return flagMap;
}

Expand Down Expand Up @@ -145,6 +148,39 @@ public void testTransparentRestart() throws Throwable {
testTransparentRestartHelper(5, SR);
}

@Test
public void testSchemaMismatchRetry() throws Throwable {
setUpTable(2, RR);
try (Connection c1 = getConnectionBuilder().connect();
Connection c2 = getConnectionBuilder().connect();
Statement s1 = c1.createStatement();
Statement s2 = c2.createStatement()) {
// Run UPDATE statement for the sole purpose of a caching catalog version.
s1.execute("UPDATE t SET v=2 WHERE k=0");
// Add more than one statement to the batch to ensure that
// YB treats this as batched execution mode.
for (int i = 1; i <= 2; i++) {
s1.addBatch(String.format("UPDATE t SET v=2 WHERE k=%d", i));
}
// Causes a schema version mismatch error on the next UPDATE statement.
// Execute ALTER in a different session c2 so as not to invalidate
// the catalog cache of c1 until the next heartbeat with the master.
s2.execute("ALTER TABLE t ALTER COLUMN v SET NOT NULL");
try {
// This uses the cached catalog version but the schema is changed
// by the ALTER TABLE statement above. This should cause a schema
// mismatch error. The schema mismatch error is not retried internally
// in batched execution mode.
s1.executeBatch();
// Should not reach here since we do not support retries in batched
// execution mode for schema mismatch errors.
fail("Internal retries are not supported in batched execution mode");
} catch (BatchUpdateException e) {
LOG.info(e.toString());
}
}
}

@Test
public void testTransparentRestartFirstStatement() throws Throwable {
// Params: conflicting statement, isolation level
Expand Down
3 changes: 3 additions & 0 deletions src/postgres/src/backend/tcop/postgres.c
Original file line number Diff line number Diff line change
Expand Up @@ -3969,9 +3969,12 @@ static void YBPrepareCacheRefreshIfNeeded(ErrorData *edata,
* transaction for future queries (before commit).
* So we just re-throw the error in that case.
*
* Do not retry statements in a batch for the same reason.
*
*/
if (consider_retry &&
!IsTransactionBlock() &&
!YbIsBatchedExecution() &&
!YBCGetDisableTransparentCacheRefreshRetry())
{
/* Clear error state */
Expand Down

0 comments on commit dd60793

Please sign in to comment.