Move AsyncFileWriteChecker to right above SimpleFile in the file stack in simulation, which is analogous to where it is created in production and prevents false positive errors caused by stacking it on top of AsyncFileNonDurable multiple times for different users of the same file.

This commit is contained in:
Steve Atherton 2023-05-03 10:39:13 -07:00
parent f3a5f434ee
commit 95be00a8be
2 changed files with 11 additions and 23 deletions

View File

@ -35,12 +35,7 @@ public:
// Lambda must hold a reference to this to keep it alive until after the read
auto self = Reference<AsyncFileWriteChecker>::addRef(this);
return map(m_f->read(data, length, offset), [self, data, offset](int r) {
// Do not check the checksum if self is the sole owner of the reference because the end user has dropped the
// file handle and may already have re-opened the file and written to it before this read completed so our
// stored checksum can be wrong.
if (!self->isSoleOwner()) {
self->updateChecksumHistory(false, offset, r, (uint8_t*)data);
}
self->updateChecksumHistory(false, offset, r, (uint8_t*)data);
return r;
});
}
@ -48,12 +43,7 @@ public:
// Lambda must hold a reference to this to keep it alive until after the read
auto self = Reference<AsyncFileWriteChecker>::addRef(this);
return map(m_f->readZeroCopy(data, length, offset), [self, data, length, offset](Void r) {
// Do not check the checksum if self is the sole owner of the reference because the end user has dropped the
// file handle and may already have re-opened the file and written to it before this read completed so our
// stored checksum can be wrong.
if (!self->isSoleOwner()) {
self->updateChecksumHistory(false, offset, *length, (uint8_t*)data);
}
self->updateChecksumHistory(false, offset, *length, (uint8_t*)data);
return r;
});
}

View File

@ -2882,10 +2882,6 @@ Future<Reference<class IAsyncFile>> Sim2FileSystem::open(const std::string& file
auto partFile = machineCache.find(actualFilename);
if (partFile != machineCache.end()) {
Future<Reference<IAsyncFile>> f = AsyncFileDetachable::open(partFile->second.get());
if (FLOW_KNOBS->PAGE_WRITE_CHECKSUM_HISTORY > 0)
f = map(f, [=](Reference<IAsyncFile> r) {
return Reference<IAsyncFile>(new AsyncFileWriteChecker(r));
});
return f;
}
}
@ -2897,11 +2893,15 @@ Future<Reference<class IAsyncFile>> Sim2FileSystem::open(const std::string& file
// This way, they can both keep up with the time to start the next operation
auto diskParameters =
makeReference<DiskParameters>(FLOW_KNOBS->SIM_DISK_IOPS, FLOW_KNOBS->SIM_DISK_BANDWIDTH);
f = AsyncFileNonDurable::open(filename,
actualFilename,
SimpleFile::open(filename, flags, mode, diskParameters, false),
diskParameters,
(flags & IAsyncFile::OPEN_NO_AIO) == 0);
f = SimpleFile::open(filename, flags, mode, diskParameters, false);
if (FLOW_KNOBS->PAGE_WRITE_CHECKSUM_HISTORY > 0) {
f = map(f,
[=](Reference<IAsyncFile> r) { return Reference<IAsyncFile>(new AsyncFileWriteChecker(r)); });
}
f = AsyncFileNonDurable::open(
filename, actualFilename, f, diskParameters, (flags & IAsyncFile::OPEN_NO_AIO) == 0);
machineCache[actualFilename] = UnsafeWeakFutureReference<IAsyncFile>(f);
} else {
@ -2909,8 +2909,6 @@ Future<Reference<class IAsyncFile>> Sim2FileSystem::open(const std::string& file
}
f = AsyncFileDetachable::open(f);
if (FLOW_KNOBS->PAGE_WRITE_CHECKSUM_HISTORY > 0)
f = map(f, [=](Reference<IAsyncFile> r) { return Reference<IAsyncFile>(new AsyncFileWriteChecker(r)); });
if (FLOW_KNOBS->ENABLE_CHAOS_FEATURES)
f = map(f, [=](Reference<IAsyncFile> r) { return Reference<IAsyncFile>(new AsyncFileChaos(r)); });
if (flags & IAsyncFile::OPEN_ENCRYPTED)