apple / apple/foundationdb

AsyncFileCached improperly handles sequencing writes with multiple concurrent truncates

Open
#2,824 2 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.