[BUG] miniob crashes when concurrently creating tables

[xzhangxian1008]

Describe the bug
A clear and concise description of what the bug is.

** Environment**
Environment Details sometimes important

• OS Version: Linux 10-2-12-124 5.14.0-284.18.1.el9_2.x86_64
• CPU Arch(x86/arm): x86
• Compiler: clang version 13.0.0
• Others:

Fast Reproduce Steps(Required)
Steps to reproduce the behavior:

1. setup miniob
2. concurrently send insert requests to miniob with 20 threads, and each thread send 10w rows.

Expected behavior
A clear and concise description of what you expected to happen.

no crash

Actual Behavior
What is the result? picture is allowed

crash

Additional context
Add any other context about the problem here.

    #0 0x5fddf1 in std::__1::pair<std::__1::__hash_iterator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, void*>*>, bool> std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, 
std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> > >::__emplace_unique_key_args<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>&&) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/__hash_table:2116:34
    #1 0x5fd1de in std::__1::pair<std::__1::__hash_iterator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, void*>*>, bool> std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, 
std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> > >::__emplace_unique_extract_key<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> >(std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>&&, std::__1::__extract_key_first_tag) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/__hash_table:1115:14
#2 0x5fcfcf in std::__1::pair<std::__1::__hash_iterator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, void*>*>, bool> std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, 
std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> > >::__emplace_unique<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> >(std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>&&) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/__hash_table:1079:14
    #3 0x5fcbbf in std::__1::pair<std::__1::__hash_iterator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, void*>*>, bool> std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, 
std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> > >::__insert_unique<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, void>(std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>&&) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/__hash_table:1137:14
    #4 0x5e206d in std::__1::pair<std::__1::__hash_map_iterator<std::__1::__hash_iterator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, void*>*> >, bool> std::__1::unordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, DiskBufferPool*> > >::insert<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, void>(std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>&&) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/unordered_map:1141:30
    #5 0x5cf13e in BufferPoolManager::open_file(char const*, DiskBufferPool*&) /DATA/disk3/xzx/miniob/src/observer/storage/buffer/disk_buffer_pool.cpp:688:17
    #6 0x68fdb3 in Table::init_record_handler(char const*) /DATA/disk3/xzx/miniob/src/observer/storage/table/table.cpp:306:41
    #7 0x68e6f7 in Table::create(int, char const*, char const*, char const*, int, AttrInfoSqlNode const*) /DATA/disk3/xzx/miniob/src/observer/storage/table/table.cpp:113:8
    #8 0x623ff4 in Db::create_table(char const*, int, AttrInfoSqlNode const*) /DATA/disk3/xzx/miniob/src/observer/storage/db/db.cpp:92:15
    #9 0x4a519d in CreateTableExecutor::execute(SQLStageEvent*) /DATA/disk3/xzx/miniob/src/observer/sql/executor/create_table_executor.cpp:37:38
    #10 0x49f420 in CommandExecutor::execute(SQLStageEvent*) /DATA/disk3/xzx/miniob/src/observer/sql/executor/command_executor.cpp:41:23
    #11 0x49bbfb in ExecuteStage::handle_request(SQLStageEvent*) /DATA/disk3/xzx/miniob/src/observer/sql/executor/execute_stage.cpp:46:27
    #12 0x4622ad in SqlTaskHandler::handle_sql(SQLStageEvent*) /DATA/disk3/xzx/miniob/src/observer/net/sql_task_handler.cpp:80:23
    #13 0x45f018 in SqlTaskHandler::handle_event(Communicator*) /DATA/disk3/xzx/miniob/src/observer/net/sql_task_handler.cpp:37:9
    #14 0x4712de in Worker::operator()() /DATA/disk3/xzx/miniob/src/observer/net/one_thread_per_connection_thread_handler.cpp:95:29
    #15 0x46e874 in decltype(static_cast<Worker&>(fp)()) std::__1::__invoke<Worker&>(Worker&) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/type_traits:3918:1
    #16 0x46e84c in std::__1::__invoke_of<Worker&>::type std::__1::reference_wrapper<Worker>::operator()<>() const /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/__functional/reference_wrapper.h:69:16
    #17 0x46e7f4 in decltype(static_cast<std::__1::reference_wrapper<Worker>>(fp)()) std::__1::__invoke<std::__1::reference_wrapper<Worker> >(std::__1::reference_wrapper<Worker>&&) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/type_traits:3918:1
    #18 0x46e794 in void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, std::__1::reference_wrapper<Worker> >(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, std::__1::reference_wrapper<Worker> >&, std::__1::__tuple_indices<>) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/thread:280:5
    #19 0x46de2f in void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, std::__1::reference_wrapper<Worker> > >(void*) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/thread:291:5
    #20 0x7f5903c9f801 in start_thread (/usr/lib64/libc.so.6+0x9f801)
    #21 0x7f5903c3f44f in __GI___clone3 (/usr/lib64/libc.so.6+0x3f44f)


SUMMARY: AddressSanitizer: SEGV /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/__hash_table:2116:34 in std::__1::pair<std::__1::__hash_iterator<std::__1::__hash_node<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, void*>*>, bool> 
std::__1::__hash_table<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::__unordered_map_hasher<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::__unordered_map_equal<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, true>, std::__1::allocator<std::__1::__hash_value_type<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> > >::__emplace_unique_key_args<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*> >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, DiskBufferPool*>&&)

    #0 0x3a1cbc in pthread_create /root/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:207:3
    #1 0x46dd18 in std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/__threading_support:514:10
    #2 0x46d568 in std::__1::thread::thread<std::__1::reference_wrapper<Worker>, void>(std::__1::reference_wrapper<Worker>&&) /DATA/disk3/xzx/env-tiflash/sysroot/bin/../include/c++/v1/thread:307:16
    #3 0x46c2bd in Worker::start() /DATA/disk3/xzx/miniob/src/observer/net/one_thread_per_connection_thread_handler.cpp:43:19
    #4 0x468ab6 in OneThreadPerConnectionThreadHandler::new_connection(Communicator*) /DATA/disk3/xzx/miniob/src/observer/net/one_thread_per_connection_thread_handler.cpp:132:18
    #5 0x40cf7e in NetServer::accept(int) /DATA/disk3/xzx/miniob/src/observer/net/server.cpp:129:25
    #6 0x419cc5 in NetServer::serve() /DATA/disk3/xzx/miniob/src/observer/net/server.cpp:285:13
    #7 0x3ef478 in main /DATA/disk3/xzx/miniob/src/observer/main.cpp:196:13
    #8 0x7f5903c3feaf in __libc_start_call_main (/usr/lib64/libc.so.6+0x3feaf)

[xzhangxian1008]

It seems that there is data race when concurrently creating table.

[hnwyllmm]

MiniOB doesn't support creating tables or indexes concurrent. You must control it manually.
But I will appreciate that if you create a pull request to fix this and I can discuss with you about the solution.

[xzhangxian1008]

MiniOB doesn't support creating tables or indexes concurrent. You must control it manually. But I will appreciate that if you create a pull request to fix this and I can discuss with you about the solution.

okk, can you give me some hints about the solution.

[hnwyllmm]

Because github doesn't support the text send by email in markdown format, I rewrite it in github.

There are two points:

1. creating tables in one Db;
2. creating tables or indexes while modifying records in the Table, such as records insertion, deletion.

It's easy to resolve the first problem. We can just add a lock in the Db when we access the tables_ data member.
But it's a little difficult to resolve the second problem. There is one solution from other database system. We can use table lock to handle the concurrency between table schema changing and table records modifying.

Usually, we use different locking mode for thus operations.
Let's say we have four locking mode:

• exclusive. Only one worker can access the table. We can change the table schema or even drop the table if we hold this mode of lock. They also can modify the records.
• shared. Many workers can hold this mode of table lock at the same time and they can reading schema information and records from the table.
• intent exclusive. Workers who hold this mode of table lock won't change the schema of the table and they just want to modify records of the table.
• intent shared. Workers who hold this mode of table lock won't change the schema of the table and they just want to read records of the table.

Below is the locking compatible table:

Lock Mode	exclusive	shared	intent exclusive	intent shared
exclusive	X	X	X	X
shared	X	√	X	√
intent exclusive	X	X	√	√
intent shared	X	√	√	√

 As you can see, when you just want to visit the records of the table, you should get the intent lock. When you want to change the schema of the table, such as adding index, you should get the exclusive lock.

It seems shared lock is need now.

Before we want to do some actions, we should get the specific mode of lock.

You can use some other solutions as the table locking is a little complex. But please share your idea with me before you submit the Pull Request.

If you can complete this feature, I'd like to propose you to be the miniob committer.

Thanks.

[xzhangxian1008]

Because github doesn't support the text send by email in markdown format, I rewrite it in github.

There are two points:

1. creating tables in one Db;
2. creating tables or indexes while modifying records in the Table, such as records insertion, deletion.

It's easy to resolve the first problem. We can just add a lock in the Db when we access the tables_ data member. But it's a little difficult to resolve the second problem. There is one solution from other database system. We can use table lock to handle the concurrency between table schema changing and table records modifying.

Usually, we use different locking mode for thus operations. Let's say we have four locking mode:

• exclusive. Only one worker can access the table. We can change the table schema or even drop the table if we hold this mode of lock. They also can modify the records.
• shared. Many workers can hold this mode of table lock at the same time and they can reading schema information and records from the table.
• intent exclusive. Workers who hold this mode of table lock won't change the schema of the table and they just want to modify records of the table.
• intent shared. Workers who hold this mode of table lock won't change the schema of the table and they just want to read records of the table.

Below is the locking compatible table:

Lock Mode exclusive shared intent exclusive intent shared
exclusive X X X X
shared X √ X √
intent exclusive X X √ √
intent shared X √ √ √
 As you can see, when you just want to visit the records of the table, you should get the intent lock. When you want to change the schema of the table, such as adding index, you should get the exclusive lock.

It seems shared lock is need now.

Before we want to do some actions, we should get the specific mode of lock.

You can use some other solutions as the table locking is a little complex. But please share your idea with me before you submit the Pull Request.

If you can complete this feature, I'd like to propose you to be the miniob committer.

Thanks.

Are there any blogs or paper that introduce the implemention of the online index?

[hnwyllmm]

Sure, here it is innodb intention locks and some other pages:
https://severalnines.com/blog/understanding-lock-granularity-mysql/

SQL server: https://sqlundercover.com/2019/07/25/intent-locks-in-sql-server/

PostgreSQL: https://habr.com/en/companies/postgrespro/articles/500714/

ARIES: A Transaction Recovery Method Supporting Fine-Granularity Locking and Partial Rollbacks Using Write-Ahead Logging

[xzhangxian1008]

Sure, here it is innodb intention locks and some other pages: https://severalnines.com/blog/understanding-lock-granularity-mysql/

SQL server: https://sqlundercover.com/2019/07/25/intent-locks-in-sql-server/

PostgreSQL: https://habr.com/en/companies/postgrespro/articles/500714/

ARIES: A Transaction Recovery Method Supporting Fine-Granularity Locking and Partial Rollbacks Using Write-Ahead Logging

get it

[xzhangxian1008]

This is a simple design doc, many things may not be considered or described. Hope for your suggestions, I will improve it. PTAL. @hnwyllmm

[hnwyllmm]

Sorry that I response your message so late. I have read your document and I list the comments below in Chinese.

If you want, you can copy the document to oceanbase yuque. Tell me if you don't have the permission.

1. 你的方案描述了各种索引增删与记录增删改查的冲突场景，不过方案也适用于增加字段、减少字段等场景；
2. LockType 看起来是一个枚举值，但是看不到其它枚举值的含义；
3. 锁相关的成员对象中使用到了table_name，现在可以改成 table id 了，你当时写文档时代码中应该还没有 table id的概念；
4. 通过Lock的实现描述来看，这里的锁并发控制，已经可以细化到页面和记录了。使用页面和记录锁来做并发控制，并不适用于索引等表结构变更与记录/行数据修改的并发控制。
5. 在“创建索引的流程“中的图中描述的，创建索引时，需要遍历获取所有的记录，来更新新索引的数据。但是使用了页面和记录锁，就存在一个缺陷：加了第1个页面之后，更新了索引数据，再加第2个页面，更新索引数据，这时是否需要释放第1个页面的锁？另外，页面加的是IS锁，其它事务是否可以在这个页面中增删记录？通常意向读锁不阻止其它事务更新数据；
6. 文档中没有给出，包括在”创建索引的流程“图中，创建索引/删除索引时，如果要增删改查行数据记录，锁怎么加；
7. 文档中没有描述，各个类型的锁，兼容性是如何的。
8. 建议给一些典型的应用场景，比如增加索引与增删查数据是如何并发的；
9. 建议简化整体设计，意向锁是与表锁一起工作的，意向锁可以防止在表数据增删改时变更表结构。另外我还了解到异步变更表结构的设计，如果有兴趣也可以一起看看，希望能开拓你的视野。

如果有兴趣可以加我微信(hnwyllmm_126)，我们可以在miniob开发群中大家一起讨论。

[xzhangxian1008]

4. 用于索引等表结构变

好的 我会做相应修改