Convert many VirtualFile APIs to async

[arpad-m]

Problem

VirtualFile does both reading and writing, and it would be nice if both could be converted to async, so that it doesn't have to support an async read path and a blocking write path (especially for the locks this is annoying as none of the lock implementations in std, tokio or parking_lot have support for both async and blocking access).

Summary of changes

This PR is some initial work on making the VirtualFile APIs async. It can be reviewed commit-by-commit.

• Introduce the MaybeVirtualFile enum to be generic in a test that compares real files with virtual files.
• Make various APIs of VirtualFile async, including write_all_at, read_at, read_exact_at.

Part of #4743 , successor of #5180.

Checklist before requesting a review

• I have performed a self-review of my code.
• If it is a core feature, I have added thorough tests.
• Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
• If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

• Do not forget to reformat commit message to not include the above checklist

[github-actions]

1624 tests run: 1550 passed, 0 failed, 74 skipped (full report)

---

Flaky tests (1)

Postgres 14

• test_get_tenant_size_with_multiple_branches: debug

_{The comment gets automatically updated with the latest test results
021c0bc at 2023-09-04T14:47:59.946Z :recycle:}

[arpad-m]

This failure is legitimate (I can reproduce it locally):

thread 'virtual_file::tests::test_virtual_files' panicked at 'assertion failed: file_a.read_string().await.is_err()', pageserver/src/virtual_file.rs:680:9
stack backtrace:

I don't know where the problem is from, as the OpenOptions are being passed through, so the file should theoretically be open as write-only. Need to investigate this more starting Monday.

[jcsp]

Totally reasonable, it was a bug that any of these were sync to begin with :-)

[arpad-m]

So looking at strace output, with some debug information, we get for the working native file:

[pid 155241] openat(AT_FDCWD, "../tmp_check/test_physical_files/file_a", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 9
[pid 155241] write(9, "foobar", 6)      = 6
[pid 155241] fcntl(9, F_GETFL)          = 0x8001 (flags O_WRONLY|O_LARGEFILE)
[pid 155241] write(1, "Native File flags are: 0x8001 of"..., 46Native File flags are: 0x8001 oflag: O_WRONLY
) = 46
[pid 155241] statx(9, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0664, stx_size=6, ...}) = 0
[pid 155241] lseek(9, 0, SEEK_CUR)      = 6
[pid 155241] read(9, 0x7ff69800c820, 32) = -1 EBADF (Bad file descriptor)
[pid 155241] openat(AT_FDCWD, "../tmp_check/test_physical_files/file_a", O_RDONLY|O_CLOEXEC) = 10
[pid 155241] write(10, "bar", 3)        = -1 EBADF (Bad file descriptor)

And for the non-working VirtualFile:

[pid 155242] openat(AT_FDCWD, "../tmp_check/test_virtual_files/file_a", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 9
[pid 155242] fcntl(9, F_GETFL)          = 0x8001 (flags O_WRONLY|O_LARGEFILE)
[pid 155242] write(1, "Open with options: 0x8001 oflag:"..., 42Open with options: 0x8001 oflag: O_WRONLY
) = 42
[pid 155242] pwrite64(9, "foobar", 6, 0) = 6
[pid 155242] statx(9, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0664, stx_size=6, ...}) = 0
[pid 155242] fcntl(9, F_GETFL)          = 0x8001 (flags O_WRONLY|O_LARGEFILE)
[pid 155242] write(1, "VirtualFile File flags are (len "..., 59VirtualFile File flags are (len 0): 0x8001 oflag: O_WRONLY
) = 59
[pid 155242] write(2, "thread '", 8thread ')    = 8
[pid 155242] write(2, "virtual_file::tests::test_virtua"..., 39virtual_file::tests::test_virtual_files) = 39
[pid 155242] write(2, "' panicked at '", 15' panicked at ') = 15
[pid 155242] write(2, "called `Result::unwrap_err()` on"..., 50called `Result::unwrap_err()` on an `Ok` value: "") = 50
[pid 155242] write(2, "', ", 3', )         = 3
[pid 155242] write(2, "pageserver/src/virtual_file.rs", 30pageserver/src/virtual_file.rs) = 30

I think the problem lies in the usage of read_at. I will try again with using normal read, i think this fixes it.

[arpad-m]

It turns out that the test failures were due to missing tracking of where we are in the file. Now, I introduced read_to_end, only used by tests, with offset tracking via self.pos, and it works. Merging once CI is green.

[arpad-m]

To expand on why I did the last commit with read_to_end, bringing the discussion out of DMs:

• The test failure one would run into at the start would be that read_string on a write-only file would succeed because the cursor would be at the end of the file, and thus there'd be zero data to read. The code would do a read call with zero size, and there is an optimization somewhere that lets that read call return an empty buffer without actually issuing a read system call (you can see above that there is no read system call happening). I first "worked around" this issue by seeking to the start of the file before doing read_string, but this is just changing the test to accomodate for behaviour changes, so not a good idea: the regression would still be there. Instead, now we do what the file API is also doing: read a non-zero piece of data, which then can still return 0, if nothing is available, but more importantly, it does a system call and thus yields the expected error.
• VirtualFile has a pos field. The code before the last commit recalculated pos via performing a seek operation, which is sub-optimal. Ideally we'd use the already existing position tracking.
• More importantly, read_string on native files affects the position inside the file, while our read_string implementation wouldn't. This was because we used read_string_at which doesn't update pos. Therefore, the test would fail where it first expected the read to return "foobar" and then to return "" (with us returning foobar both times). The read_to_end implementation now correctly updates pos (with a fine granularity even, so in error conditions we also get similar update behaviour of our cursor).