AsyncFileCached improperly handles sequencing writes with multiple concurrent truncates
- Dominant language
- C++
- Stars
- 16.7k
- Forks
- 1.6k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 126
Description
1ca98148793890a91dfe33ac5de1806d044bf81f introduced code into AsyncFileCached so that if one calls file->truncate() to extend the file, and then does a write to the extended area, that the result *must* be that the truncation occurs and then the write occurs. In AsyncFileCached, the write is forced to wait for the truncate, and another call to truncate must also wait for the ongoing truncate to finish.
The implementation of this does:
```
ACTOR static Future truncate_impl(AsyncFileCached *self, int64_t size) {
wait(self->currentTruncate);
self->currentTruncateSize = size;
self->currentTruncate = self->changeFileSize(size);
wait(self->currentTruncate);
return Void();
}
```
Which accidentally means that if one called truncate three times in a row, truncate 2 would wait on truncate 1, truncate 3 would wait on truncate 1, and then once truncate 1 finishes, truncates 2 and 3 run concurrently. sync() does not block on any of these, so truncate 3 may complete at any point in time (seconds, minutes, hours) in the future.
In making the Disk Queue use AsyncFileCached for #2525 , it unveiled this, because a BUGGIFY makes the disk queue very incrementally extend the disk queue file as it is being written, which involves many truncate calls.
I am planning to address this by continuing to model the disk as linearizable, and make a sync block on all pending operations. Specifically:
1. Calling truncate 3 times without waiting on a completed future could allow any of the 3 truncates to finish last.
2. Calling truncate twice, calling sync() and waiting on it, then calling truncate a third time will guarantee that the third truncate will finish last.
I believe that this is the minimal contract, and also makes truncate have a similar contract as read() and write().
Contributor guide
Research direction
Start with AsyncFileCached::truncate_impl and the currentTruncate/changeFileSize flow described in the issue, then inspect sync() and the Disk Queue use mentioned for #2525. The fix is done when sequential truncates are ordered and sync() waits for all pending operations, including the stated guarantee that a later truncate finishes last.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100