MDEV-34057 Inconsistent FTS state in concurrent scenarios #3250
Conversation
|
|
Thirunarayanan
force-pushed
the
10.4-MDEV-34057
branch
2 times, most recently
from
05b6468
to
1c20084
Compare
Thirunarayanan
force-pushed
the
10.4-MDEV-34057
branch
from
1c20084
to
983fc71
Compare
Thirunarayanan
force-pushed
the
10.4-MDEV-34057
branch
2 times, most recently
from
60e22f3
to
166f64b
Compare
Problem: ======= - This commit is a merge of mysql commit 129ee47ef994652081a11ee9040c0488e5275b14. InnoDB FTS can be in inconsistent state when sync operation terminates the server before committing the operation. This could lead to incorrect synced doc id and incorrect query results. Solution: ======== - During sync commit operation, InnoDB should pass the sync transaction to update the max doc id in the config table. fts_read_synced_doc_id() : This function is used to read only synced doc id from the config table.
Thirunarayanan
force-pushed
the
10.4-MDEV-34057
branch
from
166f64b
to
02ec20b
Compare
| fts_table.table = table; | ||
| fts_cache_t* cache= table->fts->cache; | ||
| dberr_t error = DB_SUCCESS; | ||
| const trx_t* const created_trx = trx; |
There was a problem hiding this comment.
The name created_trx is somewhat misleading; it could be interepreted as "created in this function", but also as "created before calling this function". A less ambiguous name would be caller_trx.
| trx_t *trx = trx_create(); | ||
| if (srv_read_only_mode) { | ||
| trx_start_internal_read_only(trx); | ||
| } else { | ||
| trx_start_internal(trx); | ||
| } | ||
| if (fts_read_synced_doc_id(table, &start_doc, trx)) { | ||
| fts_sql_rollback(trx); | ||
| trx->free(); | ||
| goto func_exit; | ||
| } | ||
| fts_sql_commit(trx); | ||
| if (start_doc) start_doc-= 1; | ||
| trx->free(); |
There was a problem hiding this comment.
Can we simplify this as follows?
trx_t* trx = trx_create();
trx_start_internal_read_only(trx);
dberr_t err = fts_read_synced_doc_id(table, &start_doc, trx);
fts_sql_commit(trx);
trx->free();
if (err != DB_SUCCESS) {
goto func_exit;
}
if (start_doc) {
start_doc--;
}
| if (srv_read_only_mode) { | ||
| trx_start_internal_read_only(trx); | ||
| } else { | ||
| trx_start_internal(trx); | ||
| } |
There was a problem hiding this comment.
If we are creating a transaction here, it would always appear to be a read-only operation, hence we should not call trx_start_internal(trx).
| source include/have_innodb.inc; | ||
| source include/have_debug.inc; | ||
| source include/not_valgrind.inc; |
There was a problem hiding this comment.
Add also have_debug_sync.inc and not_embedded.inc. not_valgrind.inc should not be strictly needed, because we are not killing the server with DBUG_SUICIDE but with an external signal while the process is blocked in DEBUG_SYNC.
Description
Fix:
in the config table.
How can this PR be tested?
./mtr innodb_fts.fts_sync_commit_resiliency
Basing the PR against the correct MariaDB version
PR quality check