Append all characters not captured by xsputn() in overflow() function (#7991)

Summary:
In the adapter class `WritableFileStringStreamAdapter`, which wraps WritableFile to be used for std::ostream, previouly only `std::endl` is considered a special case because `endl` is written by `os.put()` directly without going through `xsputn()`. `os.put()` will call `sputc()` and if we further check the internal implementation of `sputc()`, we will see it is
```
int_type __CLR_OR_THIS_CALL sputc(_Elem _Ch) {  // put a character
    return 0 < _Pnavail() ? _Traits::to_int_type(*_Pninc() = _Ch) : overflow(_Traits::to_int_type(_Ch));
```
As we explicitly disabled buffering, _Pnavail() is always 0. Thus every write, not captured by xsputn, becomes an overflow.

When I run tests on Windows, I found not only `std::endl` will drop into this case, writing an unsigned long long will also call `os.put()` then followed by `sputc()` and eventually call `overflow()`. Therefore, instead of only checking `std::endl`, we should try to append other characters as well unless the appending operation fails.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7991

Reviewed By: jay-zhuang

Differential Revision: D26615692

Pulled By: ajkr

fbshipit-source-id: 4c0003de1645b9531545b23df69b000e07014468
main
xinyuliu 4 years ago committed by Facebook GitHub Bot
parent cd79a00903
commit b085ee13e0
  1. 3
      HISTORY.md
  2. 20
      table/block_based/block_based_table_reader.h

@ -1,5 +1,8 @@
# Rocksdb Change Log # Rocksdb Change Log
## Unreleased ## Unreleased
### Bug Fixes
* Fixed the truncation error found in APIs/tools when dumping block-based SST files in a human-readable format. After fix, the block-based table can be fully dumped as a readable file.
### Public API change ### Public API change
* Add a new option BlockBasedTableOptions::max_auto_readahead_size. RocksDB does auto-readahead for iterators on noticing more than two reads for a table file if user doesn't provide readahead_size. The readahead starts at 8KB and doubles on every additional read upto max_auto_readahead_size and now max_auto_readahead_size can be configured dynamically as well. Found that 256 KB readahead size provides the best performance, based on experiments, for auto readahead. Experiment data is in PR #3282. If value is set 0 then no automatic prefetching will be done by rocksdb. Also changing the value will only affect files opened after the change. * Add a new option BlockBasedTableOptions::max_auto_readahead_size. RocksDB does auto-readahead for iterators on noticing more than two reads for a table file if user doesn't provide readahead_size. The readahead starts at 8KB and doubles on every additional read upto max_auto_readahead_size and now max_auto_readahead_size can be configured dynamically as well. Found that 256 KB readahead size provides the best performance, based on experiments, for auto readahead. Experiment data is in PR #3282. If value is set 0 then no automatic prefetching will be done by rocksdb. Also changing the value will only affect files opened after the change.

@ -656,13 +656,21 @@ class WritableFileStringStreamAdapter : public std::stringbuf {
explicit WritableFileStringStreamAdapter(WritableFile* writable_file) explicit WritableFileStringStreamAdapter(WritableFile* writable_file)
: file_(writable_file) {} : file_(writable_file) {}
// This is to handle `std::endl`, `endl` is written by `os.put()` directly // Override overflow() to handle `sputc()`. There are cases that will not go
// without going through `xsputn()`. As we explicitly disabled buffering, // through `xsputn()` e.g. `std::endl` or an unsigned long long is written by
// every write, not captured by xsputn, is an overflow. // `os.put()` directly and will call `sputc()` By internal implementation:
// int_type __CLR_OR_THIS_CALL sputc(_Elem _Ch) { // put a character
// return 0 < _Pnavail() ? _Traits::to_int_type(*_Pninc() = _Ch) :
// overflow(_Traits::to_int_type(_Ch));
// }
// As we explicitly disabled buffering (_Pnavail() is always 0), every write,
// not captured by xsputn(), becomes an overflow here.
int overflow(int ch = EOF) override { int overflow(int ch = EOF) override {
if (ch == '\n') { if (ch != EOF) {
file_->Append("\n"); Status s = file_->Append(Slice((char*)&ch, 1));
return ch; if (s.ok()) {
return ch;
}
} }
return EOF; return EOF;
} }

Loading…
Cancel
Save