Bug: Only one related transaction should be allowed to exist when making a DDL statement

[KKould]

Bug: Only one related transaction should be allowed to exist when making a DDL statement

[crwen]

I have some problems in adding locks for methods of Transaction. Here is my understanding:

1. read, read_by_index, append, delete, table, drop_data should add read lock
2. add_column, drop_column, create_table should add write lock
3. how about add_index, del_index and add_index_meta? I found both ddl(insert, update) and dml(create index, add column) use add_index. Maybe add_index_meta should add write lock, others should be read lock.
4. should meta-related methods be locked? Which lock should be used?(table_metas, table_meta_path, save_table_meta, meta_load)

[crwen]

It's kind of like a hierarchical lock and intention lock, but it is still a table level lock in most case, and multiple ddl on different table can execute concurrently.

session1                                        session2
select * from t;
                                               alter table t2 add column d int(ok)
                                               alter table t add column d int(block)

+-------------+--------------------+----------------+---------------------+
| OBJECT_TYPE | OBJECT_SCHEMA      | OBJECT_NAME    | LOCK_TYPE           |
+-------------+--------------------+----------------+---------------------+
| TABLE       | test               | t              | SHARED_READ         | 
| GLOBAL      | NULL               | NULL           | INTENTION_EXCLUSIVE | 
| BACKUP LOCK | NULL               | NULL           | INTENTION_EXCLUSIVE |
| SCHEMA      | test               | NULL           | INTENTION_EXCLUSIVE |
| TABLE       | test               | t              | SHARED_UPGRADABLE   |
| TABLESPACE  | NULL               | test/t         | INTENTION_EXCLUSIVE |
| TABLE       | test               | #sql-790d_b    | EXCLUSIVE           | 
| TABLE       | test               | t              | EXCLUSIVE           |
| TABLE       | performance_schema | metadata_locks | SHARED_READ         | 
+-------------+--------------------+----------------+---------------------+

I don't think implicitly submiting is a good thing, you can't even rollback(even when you kill it, it is crazy).

[crwen]

@crwen do you have any ideas for implementing table level locks? I don't have any good ideas now

I have no idea. But tables are known only when binding or after, maybe lock can be added when executing or acessing the storage.

[crwen]

I think we can first make mdl a global lock, restrict DDL to global serialization, and downgrade it to table level lock as a perf issue

So maybe #177 could be merged now.

[crwen]

maybe some lock manager?

[KKould]

Yes, it is consistent with the table lock in MySQL. When a DDL operation occurs, no other DDL or DML is allowed.

Tips: I think DML and DDL are operations similar to RWLock, DML can be run by multiple DML transactions simultaneously, but DDL runs a single DDL transaction simultaneously in DML and DDL
Tips 2: If there are duplicates in DML, Storage will detect and throw an exception by itself, This is already implemented so you don't need to worry about this

This may be a relatively big job, so before starting this issue, could you please fix the nestloopjoin pr problem first?

[crwen]

could you please fix the nestloopjoin pr problem first?

I think the main problem is in the binder layer, or are you referring to the problem of explain?

[KKould]

Oh, actually I think the latest issue you raised can be postponed: #169(I will solve this issue in the near future), and the current issues with nestloopjoin are: - Problem with `or` operator: #164 (comment) - The code you proposed and I gave the solution in another PR: #168 crwen ***@***.***> 于2024年3月19日周二 23:26写道：

…

could you please fix the nestloopjoin pr <#164> problem first? I think the main problem is in the binder layer, or are you referring to the problem of explain? — Reply to this email directly, view it on GitHub <#171 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AV2JGRBPSC2NIXU26XPAJQDYZBKJZAVCNFSM6AAAAABE3CPN6CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBXGQ4DQNBZGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[KKould]

I tested and cherry-picked the commit of PR: #168 to your PR. SQLite has been able to pass normally. If you are inconvenient now, let me add it?

[KKould]

I tried to implement it today and discovered something. DDL is executed as a separate transaction in MySQL. If a transaction has not yet been submitted after executing DQL or DML, etc., executing DDL at this time will implicitly submit the transaction. And to do a separate transaction for the DDL (which means only using RWLock), my plan is to add a new attribute to the DataBase

pub struct Database<S: Storage> {
    pub storage: S,
    functions: Arc<Functions>,
    mdl: Arc<RwLock<()>>
}

I chose to learn MySQL and do it on the upper layer of the storage engine, and table_cache is based on your PR

[crwen]

If a transaction has not yet been submitted after executing DQL or DML, etc., executing DDL at this time will implicitly submit the transaction.

I think DDL will be blocked if a DQL or DML does not submit. (MySQL 8.3.0)

my plan is to add a new attribute to the DataBase

This will lock the whole database.

[KKould]

yes, I originally meant within the same transaction, but if it is multiple transactions, then it is what you said

[KKould]

This will lock the whole database.

yes, so it is guaranteed that no other operations are allowed to be performed when DDL is executed
In MySQL, mdl often produces deadlocks

[crwen]

In MySQL, mdl often produces deadlocks

As far as I know, mdl is table level lock, and it is 2PL that causes deadlocks.

[KKould]

In MySQL, mdl often produces deadlocks

As far as I know, mdl is table level lock, and it is 2PL that causes deadlocks.

https://stackoverflow.com/questions/76484118/why-mysql-mdlmeta-data-lock-led-to-a-deadlock

[KKould]

Yes, you are right, so I did not do implicit commit in pr, but refused to execute DDL in transaction

[KKould]

Oh, I made a mistake, I should have used table level locks

[KKould]

@crwen do you have any ideas for implementing table level locks? I don't have any good ideas now

[KKould]

I think we can first make mdl a global lock, restrict DDL to global serialization, and downgrade it to table level lock as a perf issue