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.

Sign up for GitHub

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

Jump to bottom

[Bug Report] two_write_queue is conflict with IngestExternalFile #5974

Closed
Little-Wallace opened this issue Oct 28, 2019 · 7 comments
Closed

[Bug Report] two_write_queue is conflict with IngestExternalFile #5974

Little-Wallace opened this issue Oct 28, 2019 · 7 comments
Assignees
@maysamyabandeh

Comments

@Little-Wallace
Copy link
Contributor

Little-Wallace commented Oct 28, 2019 •
edited
Loading

Note: Please use Issues only for bug reports. For questions, discussions, feature requests, etc. post to dev group: https://www.facebook.com/groups/rocksdb.dev

Expected behavior

I'm trying to replace DBImpl with TransactionDB to improve our project performance, following the method described in this blog: unordered_write.
I enable the param unordered_write and two_write_queue to improve performance of write with multiple thread. We expect TransactionDB could perform as DBImpl.

Actual behavior

When I ingest one sstable file into TransactionDB, it was blocked.

Steps to reproduce the behavior

Here is my code to reproduce the problem: https://github.com/Little-Wallace/rocksdb/blob/transaction_db/utilities/transactions/write_prepared_transaction_test.cc#L3522

I ingest one sstable which overlap with key range of memtable. Of course, it will trigger one FlushJob of memtable.

I have run pstack for the process, and I found that RocksDB waited twice to be leader in nonmem_write_thread_. One leader thread would be blocked if it tries to become leader in a second time.
Here is the first time to EnterUnbatched : https://github.com/facebook/rocksdb/blob/master/db/db_impl/db_impl.cc#L3817

And the second time: https://github.com/facebook/rocksdb/blob/master/db/db_impl/db_impl_write.cc#L1581
@Little-Wallace Little-Wallace changed the title [Bug] two_write_queue is conflict with IngestExternalFile [Bug Report] two_write_queue is conflict with IngestExternalFile Oct 28, 2019
@Little-Wallace
Copy link
Contributor Author

I thank we could use one thread_local variable here to avoid �EnterUnbatched twice.

@Little-Wallace Little-Wallace mentioned this issue Oct 28, 2019
Closed
@maysamyabandeh
Copy link
Contributor

@yiwu-arbug Is this the same problem you faced?

@siying
Copy link
Contributor

siying commented Oct 29, 2019

btw @yiwu-arbug can you take a look at #5716 ?

@siying
Copy link
Contributor

siying commented Oct 29, 2019

@maysamyabandeh @Little-Wallace is @yiwu-arbug 's teammate. So the answer to your question must be no.

@yiwu-arbug
Copy link
Contributor

@yiwu-arbug Is this the same problem you faced?

Yes, I was referring to this issue found by @Little-Wallace.

@yiwu-arbug
Copy link
Contributor

btw @yiwu-arbug can you take a look at #5716 ?

Sorry it fell off my radar. Will take a look.

@yiwu-arbug
Copy link
Contributor

Shall we enable unordered_write in db_crashtest.py?

@maysamyabandeh maysamyabandeh self-assigned this Nov 5, 2019
merryChris pushed a commit to merryChris/rocksdb that referenced this issue Nov 18, 2019
@Little-Wallace @facebook-github-bot
Fix IngestExternalFile's bug with two_write_queue (facebook#5976)
464a20a 
Summary:
When two_write_queue enable, IngestExternalFile performs EnterUnbatched on both write queues. SwitchMemtable also EnterUnbatched on 2nd write queue when this option is enabled. When the call stack includes IngestExternalFile -> FlushMemTable -> SwitchMemtable, this results into a deadlock.
The implemented solution is to pass on the existing writes_stopped argument in FlushMemTable to skip EnterUnbatched in SwitchMemtable.
Fixes facebook#5974
Pull Request resolved: facebook#5976

Differential Revision: D18535943

Pulled By: maysamyabandeh

fbshipit-source-id: a4f9d4964c10d4a7ca06b1e0102ca2ec395512bc
yiwu-arbug pushed a commit to tikv/rocksdb that referenced this issue Nov 26, 2019
@Little-Wallace
Fix IngestExternalFile's bug with two_write_queue (facebook#5976)
1657021 
Summary:
When two_write_queue enable, IngestExternalFile performs EnterUnbatched on both write queues. SwitchMemtable also EnterUnbatched on 2nd write queue when this option is enabled. When the call stack includes IngestExternalFile -> FlushMemTable -> SwitchMemtable, this results into a deadlock.
The implemented solution is to pass on the existing writes_stopped argument in FlushMemTable to skip EnterUnbatched in SwitchMemtable.
Fixes facebook#5974
Pull Request resolved: facebook#5976

Differential Revision: D18535943

Pulled By: maysamyabandeh

fbshipit-source-id: a4f9d4964c10d4a7ca06b1e0102ca2ec395512bc
yiwu-arbug pushed a commit to tikv/rocksdb that referenced this issue Nov 27, 2019
@Little-Wallace
Fix IngestExternalFile's bug with two_write_queue (facebook#5976)
ae2c5a2 
Summary:
When two_write_queue enable, IngestExternalFile performs EnterUnbatched on both write queues. SwitchMemtable also EnterUnbatched on 2nd write queue when this option is enabled. When the call stack includes IngestExternalFile -> FlushMemTable -> SwitchMemtable, this results into a deadlock.
The implemented solution is to pass on the existing writes_stopped argument in FlushMemTable to skip EnterUnbatched in SwitchMemtable.
Fixes facebook#5974
Pull Request resolved: facebook#5976

Differential Revision: D18535943

Pulled By: maysamyabandeh

fbshipit-source-id: a4f9d4964c10d4a7ca06b1e0102ca2ec395512bc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maysamyabandeh maysamyabandeh

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix IngestExternalFile's bug with two_write_queue Little-Wallace/rocksdb
4 participants
@maysamyabandeh @yiwu-arbug @siying @Little-Wallace