diff --git a/HISTORY.md b/HISTORY.md index 941b4e510..8d373ceaa 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -48,6 +48,9 @@ * Extended parameter `output_file_names` of `CompactFiles` API to also include paths of the blob files generated by the compaction in Integrated BlobDB. * Most `BackupEngine` functions now return `IOStatus` instead of `Status`. Most existing code should be compatible with this change but some calls might need to be updated. +### Miscellaneous +* Add a paranoid check where in case FileSystem layer doesn't fill the buffer but returns succeed, checksum is unlikely to match even if buffer contains a previous block. The byte modified is not useful anyway, so it isn't expected to change any behavior when FileSystem is satisfying its contract. + ## 6.24.0 (2021-08-20) ### Bug Fixes * If the primary's CURRENT file is missing or inaccessible, the secondary instance should not hang repeatedly trying to switch to a new MANIFEST. It should instead return the error code encountered while accessing the file. diff --git a/file/random_access_file_reader.cc b/file/random_access_file_reader.cc index 9bad958c4..dd0392609 100644 --- a/file/random_access_file_reader.cc +++ b/file/random_access_file_reader.cc @@ -41,6 +41,15 @@ IOStatus RandomAccessFileReader::Read(const IOOptions& opts, uint64_t offset, (void)aligned_buf; TEST_SYNC_POINT_CALLBACK("RandomAccessFileReader::Read", nullptr); + + // To be paranoid: modify scratch a little bit, so in case underlying + // FileSystem doesn't fill the buffer but return success and `scratch` returns + // contains a previous block, returned value will not pass checksum. + if (n > 0 && scratch != nullptr) { + // This byte might not change anything for direct I/O case, but it's OK. + scratch[0]++; + } + IOStatus io_s; uint64_t elapsed = 0; { @@ -214,6 +223,18 @@ IOStatus RandomAccessFileReader::MultiRead(const IOOptions& opts, AlignedBuf* aligned_buf) const { (void)aligned_buf; // suppress warning of unused variable in LITE mode assert(num_reqs > 0); + + // To be paranoid modify scratch a little bit, so in case underlying + // FileSystem doesn't fill the buffer but return succee and `scratch` returns + // contains a previous block, returned value will not pass checksum. + // This byte might not change anything for direct I/O case, but it's OK. + for (size_t i = 0; i < num_reqs; i++) { + FSReadRequest& r = read_reqs[i]; + if (r.len > 0 && r.scratch != nullptr) { + r.scratch[0]++; + } + } + IOStatus io_s; uint64_t elapsed = 0; { diff --git a/file/sequence_file_reader.cc b/file/sequence_file_reader.cc index 70a0e0f07..614dbb411 100644 --- a/file/sequence_file_reader.cc +++ b/file/sequence_file_reader.cc @@ -58,6 +58,14 @@ IOStatus SequentialFileReader::Read(size_t n, Slice* result, char* scratch) { *result = Slice(scratch, r); #endif // !ROCKSDB_LITE } else { + // To be paranoid, modify scratch a little bit, so in case underlying + // FileSystem doesn't fill the buffer but return succee and `scratch` + // returns contains a previous block, returned value will not pass + // checksum. + // It's hard to find useful byte for direct I/O case, so we skip it. + if (n > 0 && scratch != nullptr) { + scratch[0]++; + } io_s = file_->Read(n, IOOptions(), result, scratch, nullptr); } IOSTATS_ADD(bytes_read, result->size());