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

Remove Rdb_transaction::m_ddl_transaction #1391

Conversation

laurynas-biveinis
Copy link
Contributor

With DD tables in MyRocks, the property of whether it is safe to skip uniqueness checks on writes should be resolved on a table, not transaction level. Thus fold the logic of setting m_ddl_transaction to true into ha_rocksdb::skip_unique. At the same time this flag is duplicated between struct update_row_info and callstack of affected functions, thus remove it from the callstack.

@laurynas-biveinis
Copy link
Contributor Author

This is done in preparation for always flushing DD transactions: it will require a new flag m_dd_transaction or similar in Rdb_transaction, which would have too similar a name yet completely different functionality from m_ddl_transaction. Thus removing the latter first.

@luqun luqun requested a review from lth November 15, 2023 23:00
With DD tables in MyRocks, the property of whether it is safe to skip uniqueness
checks on writes should be resolved on a table, not transaction level. Thus fold
the logic of setting m_ddl_transaction to true into ha_rocksdb::skip_unique. At
the same time this flag is duplicated between struct update_row_info and
callstack of affected functions, thus remove it from the callstack.
@laurynas-biveinis
Copy link
Contributor Author

@luqun , removed TABLE_CATEGORY_TEMPORARY and rebased; ready for review

@facebook-github-bot
Copy link

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

1 similar comment
@facebook-github-bot
Copy link

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

@luqun
Copy link
Contributor

luqun commented Nov 22, 2023

looks like rocksdb.rqg_runtime MTR failed. still investigating

@luqun
Copy link
Contributor

luqun commented Nov 25, 2023

looks like rocksdb.rqg_runtime MTR failed. still investigating

The issue is related to DDL skip uniq check, such as following code. Add PK should fail due to duplicate col2 value.

create table t1(col1 int, col2 int, col3 varchar(16));
insert into t1 values(0,0,'0');
insert into t1 values(1,0,'0');
ALTER TABLE t1 ADD KEY ( col1 );
ALTER TABLE t1 ADD PRIMARY KEY ( col2 );
check table t1;

@laurynas-biveinis
Copy link
Contributor Author

The DDL transaction flag cannot be folded into unique check flag, the original design idea was broken. Thinking of alternatives

@laurynas-biveinis
Copy link
Contributor Author

The DD durability PR is #1403. I will scavenge non-functional skip_unique_check cleanups from this PR

@laurynas-biveinis
Copy link
Contributor Author

The cleanups are in #1404

@laurynas-biveinis laurynas-biveinis deleted the rm-tx-ddl-flag branch December 4, 2023 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants