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

Endless loop in function WritableFileWriter::Append #7109

Closed
majianpeng opened this issue Jul 10, 2020 · 0 comments
Closed

Endless loop in function WritableFileWriter::Append #7109

majianpeng opened this issue Jul 10, 2020 · 0 comments
Assignees
@zhichao-cao @riversand963
Labels
bug Confirmed RocksDB bugs

Comments

@majianpeng
Copy link
Contributor

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

Expected behavior

Actual behavior

endless loop.

Steps to reproduce the behavior

set rocksdb options: use_direct_io_for_flush_and_compaction=true &&writable_file_max_buffer_size=0. This make endless loop in
WritableFileWriter::Append
if (use_direct_io() || (buf_.Capacity() >= left)) {
while (left > 0) {
size_t appended = buf_.Append(src, left);

because writable_file_max_buffer_size=0, this make buf_.Capacity always is zero. So appened is zero. This make endless loop.
@zhichao-cao zhichao-cao self-assigned this Mar 23, 2021
@zhichao-cao zhichao-cao added the bug Confirmed RocksDB bugs label Mar 23, 2021
@riversand963 riversand963 self-assigned this Dec 30, 2021
@riversand963 riversand963 mentioned this issue Dec 30, 2021
Closed
@riversand963 riversand963 linked a pull request Dec 30, 2021 that will close this issue
Disallow a combination of options #9348
Closed
facebook-github-bot pushed a commit that referenced this issue Jan 28, 2022
@riversand963 @facebook-github-bot
Disallow a combination of options (#9348)
dd203ed 
Summary:
Disallow `immutable_db_opts.use_direct_io_for_flush_and_compaction == true` and
`mutable_db_opts.writable_file_max_buffer_size == 0`, since it causes `WritableFileWriter::Append()`
to loop forever and does not make much sense in direct IO.

This combination of options itself does not make much sense: asking RocksDB to do direct IO but not allowing
RocksDB to allocate a buffer. We should detect this false combination and warn user early, no matter whether
the application is running on a platform that supports direct IO or not. In the case of platform **not** supporting
direct IO, it's ok if the user learns about this and then finds that direct IO is not supported.

One tricky thing: the constructor of `WritableFileWriter` is being used in our unit tests, and it's impossible
to return status code from constructor. Since we do not throw, I put an assertion for now. Fortunately,
the constructor is not exposed to external applications.

Closing #7109

Pull Request resolved: #9348

Test Plan: make check

Reviewed By: ajkr

Differential Revision: D33371924

Pulled By: riversand963

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

@zhichao-cao zhichao-cao

@riversand963 riversand963

Labels
bug Confirmed RocksDB bugs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Disallow a combination of options riversand963/rocksdb
3 participants
@majianpeng @zhichao-cao @riversand963