Make MyRocks DD commits always durable

[laurynas-biveinis]

Regular commit durability is governed by the binlog group commit algorithm and system variables. Make any commits (and prepares in 2PC) involving DD tables always durable unconditionally, just like InnoDB does. It is implemented through a new flag Rdb_transaction::m_dd_transaction, which is set in ha_rocksdb::external_lock and checked in rocksdb_prepare. To avoid confusion, rename the existing m_ddl_transaction flag to m_bulk_index_transaction.

[facebook-github-bot]

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[luqun]

how about check thd_is_dd_update_stmt() during trx prepare(or m_ddl_transaction = thd_is_dd_update_stmt(thd))? so that we have similar behavior as innodb?

such as

  if (thd_is_dd_update_stmt(thd)) {
    tx->set_sync(true);
    
    or 
    
    m_ddl_transaction = thd_is_dd_update_stmt(thd);
    
    Related innodb code: https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.32/storage/innobase/trx/trx0trx.cc#L2034

[laurynas-biveinis]

how about check thd_is_dd_update_stmt() during trx prepare(or m_ddl_transaction = thd_is_dd_update_stmt(thd))?

Unfortunately thd_is_dd_update_stmt is true in fewer instances than ha_rocksdb::external_lock(F_WRLCK) is called on a table with a DD category and making this change re-introduces crash recovery test failures. For InnoDB, thd_is_dd_update_stmt is also not the only input for trx->ddl_operation.

Some examples of queries where the two differ, test rocksdb_dd_innodb.ddl_crash_basic:

Mismatch in external_lock: table_share->table_category == TABLE_CATEGORY_DICTIONARY = 1, thd_is_dd_update_stmt() = 1, query = OPTIMIZE TABLE t1
Mismatch in rocksdb_prepare: is_dd_update = 0, tx->is_dd_transaction() = 1, query = OPTIMIZE TABLE t1
Mismatch in external_lock: table_share->table_category == TABLE_CATEGORY_DICTIONARY = 1, thd_is_dd_update_stmt() = 1, query = DROP TABLE t1
Mismatch in rocksdb_prepare: is_dd_update = 0, tx->is_dd_transaction() = 1, query = DROP TABLE t1
Mismatch in external_lock: table_share->table_category == TABLE_CATEGORY_DICTIONARY = 1, thd_is_dd_update_stmt() = 1, query = CREATE TABLESPACE ts1 ADD DATAFILE 'ts1.ibd'
Mismatch in rocksdb_prepare: is_dd_update = 0, tx->is_dd_transaction() = 1, query = CREATE TABLESPACE ts1 ADD DATAFILE 'ts1.ibd'
Mismatch in external_lock: table_share->table_category == TABLE_CATEGORY_DICTIONARY = 1, thd_is_dd_update_stmt() = 1, query = ALTER TABLESPACE ts1 RENAME TO ts1_renamed
Mismatch in external_lock: table_share->table_category == TABLE_CATEGORY_DICTIONARY = 1, thd_is_dd_update_stmt() = 1, query = ALTER TABLESPACE ts1 RENAME TO ts1_renamed
Mismatch in rocksdb_prepare: is_dd_update = 0, tx->is_dd_transaction() = 1, query = ALTER TABLESPACE ts1 RENAME TO ts1_renamed

[laurynas-biveinis]

There are also some cases where thd_is_dd_update_stmt is set, but no DD table is write-locked in MyRocks: CREATE EVENT event_t1 ... which accesses a system, not a DD table.

[facebook-github-bot]

@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.

[laurynas-biveinis]

@luqun, rebased with no substantial changes (for now?), added a small tweak to ha_rocksdb::is_dd_update not to check default_dd_system_storage_engine.

[facebook-github-bot]

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.