Make WritableFileWrapper not screw up preallocation

Summary:
Without this diff, this is what happens to compaction output file if it's a subclass of WritableFileWrapper:
- during compaction, all `PrepareWrite()` calls update `last_preallocated_block_` of the `WritableFileWrapper` itself, not of `target_`, since `PrepareWrite()` is not virtual,
- `PrepareWrite()` calls `Allocate()`, which is virtual; it does `fallocate()` on `target_`,
- after writing data, `target_->Close()` calls `GetPreallocationStatus()` of `target_`; it returns `last_preallocated_block_` of `target_`, which is zero because it was never touched before,
- `target_->Close()` doesn't call `ftruncate()`; file remains big.

This diff fixes it in a straightforward way, by making the methods virtual. `WritableFileWrapper` ends up having the useless fields `last_preallocated_block_` and `preallocation_block_size_`. I think ideally the preallocation logic should be outside `WritableFile`, the same way as `log_writer.h` and `file_reader_writer.h` moved some non-platform-specific logic out of Env, but that's probably not worth the effort now.

Test Plan: `make -j check`; I'm going to deploy it on our test tier and see if it fixes space reclamation problem there

Reviewers: yhchiang, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, sdong

Differential Revision: https://reviews.facebook.net/D54681
This commit is contained in:
Mike Kolupaev 2016-03-23 09:14:56 -07:00
parent ec458dcdeb
commit 4e85b74790

View File

@ -561,7 +561,7 @@ class WritableFile {
* underlying storage of a file (generally via fallocate) if the Env
* instance supports it.
*/
void SetPreallocationBlockSize(size_t size) {
virtual void SetPreallocationBlockSize(size_t size) {
preallocation_block_size_ = size;
}
@ -597,7 +597,7 @@ class WritableFile {
// of space on devices where it can result in less file
// fragmentation and/or less waste from over-zealous filesystem
// pre-allocation.
void PrepareWrite(size_t offset, size_t len) {
virtual void PrepareWrite(size_t offset, size_t len) {
if (preallocation_block_size_ == 0) {
return;
}
@ -950,6 +950,13 @@ class WritableFileWrapper : public WritableFile {
return target_->InvalidateCache(offset, length);
}
virtual void SetPreallocationBlockSize(size_t size) override {
target_->SetPreallocationBlockSize(size);
}
virtual void PrepareWrite(size_t offset, size_t len) override {
target_->PrepareWrite(offset, len);
}
protected:
Status Allocate(uint64_t offset, uint64_t len) override {
return target_->Allocate(offset, len);