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

Bad assertion in DBImpl::MarkLogsSynced() #872

Closed
bdbyrne opened this issue Dec 7, 2015 · 5 comments
Closed

Bad assertion in DBImpl::MarkLogsSynced() #872

bdbyrne opened this issue Dec 7, 2015 · 5 comments

Comments

@bdbyrne
Copy link

bdbyrne commented Dec 7, 2015

I'm hitting the following assertion in DBImpl::MarkLogsSynced():
assert(logs_.empty() || (logs_.size() == 1 && !logs_[0].getting_synced));

This appears to be due to a DBImpl::SyncWAL() call that sets exactly 0 logs to getting_synced = true, because the loop breaks immediately from condition it->number <= current_log_number failing. The mutex is unlocked, during which time another DBImpl::SyncWAL() is called that marks the first entry with getting_synced = true. The first call reacquires the mutex, then enters DBImple::MarkLogSynced(), tripping on the assertion.

I would appreciate someone verifying the scenario is valid and performing the appropriate fix.

@siying
Copy link
Contributor

siying commented Feb 22, 2016

Let me take a look.

@siying
Copy link
Contributor

siying commented Feb 23, 2016

@bdbyrne other than SyncWAL(), do you do normal sync writes?

@siying
Copy link
Contributor

siying commented Feb 23, 2016

I don't quite understand what you described, but I can repro the assert failure by having SyncWAL() run concurrently with log rolling: https://reviews.facebook.net/D54621

Is there a way you can verify whether it is the problem you ran into?

@bdbyrne
Copy link
Author

bdbyrne commented Feb 23, 2016

Yes, normal sync writes are used quite heavily in my environment, where various updates are required to be persistent before being acknowledged.

Your fix is similar to mine, so I can say it resolves my issue. Thanks for investigating, @siying.

@gfosco
Copy link
Contributor

gfosco commented Jan 10, 2018

Closing this via automation due to lack of activity. If discussion is still needed here, please re-open or create a new/updated issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants