Skip to content

Commit c007dd1

Browse files
Xingbo Wangjaykorean
authored andcommitted
Switch back to FSWritableFile in external sst file ingestion job (#13791)
Summary: This patch reverted "NewRandomRWFile" back to "ReopenWritableFile" in external sst file ingestion job when file is linked instead of copied. The reason is that some of the file systems do not support "NewRandomRWFile". A long term fix is being worked in progress. Pull Request resolved: #13791 Test Plan: Unit test Reviewed By: pdillinger Differential Revision: D78697825 Pulled By: xingbowang fbshipit-source-id: d3651223ab1f2369aac34b772bba8049c6c2c628
1 parent 0d79386 commit c007dd1

File tree

2 files changed

+41
-10
lines changed

2 files changed

+41
-10
lines changed

db/external_sst_file_ingestion_job.cc

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,6 @@ Status ExternalSstFileIngestionJob::Prepare(
156156
// It is unsafe to assume application had sync the file and file
157157
// directory before ingest the file. For integrity of RocksDB we need
158158
// to sync the file.
159-
// Use FSRandomRWFile instead of FSWritableFile, as in encrypted file
160-
// system the FSWritableFile will append a new prefix to the end of the
161-
// file when the file exists, which causes file corruption. On the
162-
// contrary, FSRandomRWFile handles an existing file correctly.
163159

164160
// TODO(xingbo), We should in general be moving away from production
165161
// uses of ReuseWritableFile (except explicitly for WAL recycling),
@@ -168,9 +164,9 @@ Status ExternalSstFileIngestionJob::Prepare(
168164
// re-open+sync+close combo but can (a) be reused easily, and (b) be
169165
// overridden to do that more cleanly, e.g. in EncryptedEnv.
170166
// https://github.com/facebook/rocksdb/issues/13741
171-
std::unique_ptr<FSRandomRWFile> file_to_sync;
172-
Status s = fs_->NewRandomRWFile(path_inside_db, env_options_,
173-
&file_to_sync, nullptr);
167+
std::unique_ptr<FSWritableFile> file_to_sync;
168+
Status s = fs_->ReopenWritableFile(path_inside_db, env_options_,
169+
&file_to_sync, nullptr);
174170
TEST_SYNC_POINT_CALLBACK("ExternalSstFileIngestionJob::Prepare:Reopen",
175171
&s);
176172
// Some file systems (especially remote/distributed) don't support

env/env_encryption.cc

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -665,17 +665,52 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
665665
std::unique_ptr<FSWritableFile>* result,
666666
IODebugContext* dbg) override {
667667
result->reset();
668-
if (options.use_mmap_writes) {
668+
if (options.use_mmap_reads || options.use_mmap_writes) {
669669
return IOStatus::InvalidArgument();
670670
}
671+
672+
size_t prefix_length = 0;
673+
std::unique_ptr<BlockAccessCipherStream> stream;
674+
671675
// Open file using underlying Env implementation
672676
std::unique_ptr<FSWritableFile> underlying;
673-
IOStatus status =
677+
auto status =
674678
FileSystemWrapper::ReopenWritableFile(fname, options, &underlying, dbg);
675679
if (!status.ok()) {
676680
return status;
677681
}
678-
return CreateWritableEncryptedFile(fname, underlying, options, result, dbg);
682+
683+
if (underlying->GetFileSize(options.io_options, dbg) != 0) {
684+
// read the cipher stream from file for non-empty file
685+
std::unique_ptr<FSRandomAccessFile> underlying_file_reader;
686+
status = FileSystemWrapper::NewRandomAccessFile(
687+
fname, options, &underlying_file_reader, dbg);
688+
if (!status.ok()) {
689+
return status;
690+
}
691+
692+
status = CreateRandomReadCipherStream(
693+
fname, underlying_file_reader, options, &prefix_length, &stream, dbg);
694+
695+
if (!status.ok()) {
696+
return status;
697+
}
698+
} else {
699+
// create cipher stream for new or empty file
700+
status = CreateWritableCipherStream(fname, underlying, options,
701+
&prefix_length, &stream, dbg);
702+
if (!status.ok()) {
703+
return status;
704+
}
705+
}
706+
707+
if (stream) {
708+
result->reset(new EncryptedWritableFile(
709+
std::move(underlying), std::move(stream), prefix_length));
710+
} else {
711+
result->reset(underlying.release());
712+
}
713+
return status;
679714
}
680715

681716
IOStatus ReuseWritableFile(const std::string& fname,

0 commit comments

Comments
 (0)