Use tokio locks in VirtualFile and turn with_file into macro

[arpad-m]

Problem

For #4743, we want to convert everything up to the actual I/O operations of VirtualFile to async fn.

Summary of changes

This PR is the last change in a series of changes to VirtualFile: #5189, #5190, #5195, #5203, and #5224.

It does the last preparations before the I/O operations are actually made async. We are doing the following things:

• First, we change the locks for the file descriptor cache to tokio's locks that support Send. This is important when one wants to hold locks across await points (which we want to do), otherwise the Future won't be Send. Also, one shouldn't generally block in async code as executors don't like that.
• Due to the lock change, we now take an approach for the VirtualFile destructors similar to the one proposed by page_cache: use try_write in drop_buffers_for_immutable #5122 for the page cache, to use try_write. Similarly to the situation in the linked PR, one can make an argument that if we are in the destructor and the slot has not been reused yet, we are the only user accessing the slot due to owning the lock mutably. It is still possible that we are not obtaining the lock, but the only cause for that is the clock algorithm touching the slot, which should be quite an unlikely occurence. For the instance of try_write failing, we spawn an async task to destroy the lock. As just argued however, most of the time the code path where we spawn the task should not be visited.
• Lastly, we split with_file into a macro part, and a function part that contains most of the logic. The function part returns a lock object, that the macro uses. The macro exists to perform the operation in a more compact fashion, saving code from putting the lock into a variable and then doing the operation while measuring the time to run it. We take the locks approach because Rust has no support for async closures. One can make normal closures return a future, but that approach gets into lifetime issues the moment you want to pass data to these closures via parameters that has a lifetime (captures work). For details, see this and this link. In Make VirtualFile::{open, open_with_options, create,sync_all,with_file} async fn #5224, we ran into a similar problem with the test_files function, and we ended up passing the path and the OpenOptions by-value instead of by-ref, at the expense of a few extra copies. This can be done as the data is cheaply copyable, and we are in test code. But here, we are not, and while File::try_clone exists, it issues system calls internally. Also, it would allocate an entirely new file descriptor, something that the fd cache was built to prevent.
• We change the STORAGE_IO_TIME metrics to support async.

Part of #4743.

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]

1644 tests run: 1570 passed, 0 failed, 74 skipped (full report)

---

Code coverage (full report)

• functions: 53.0% (7583 of 14311 functions)
• lines: 81.5% (44787 of 54967 lines)

---

_{The comment gets automatically updated with the latest test results
e1518e2 at 2023-09-11T14:58:24.907Z :recycle:}

[hlinnaka]

Lastly, we switch with_file to a macro. Mostly we do this because Rust has no support for async closures. One can make normal closures return a future, but that approach gets into lifetime issues the moment you want to pass data to these closures that has a lifetime.

Another alternative to closures is to introduce a new guard object. So instead of:

self.with_file("foo", |file|.sync_all()).await?

The callers would look like this:

{
    let file_guard = self.with_file();
    file_guard.sync_all().await?
    // `file_guard` is dropped
}

I guess in this case, the guard object would be just slot_guard, or a wrapper around it.

[arpad-m]

The idea with the guards was really great, I've moved most of the logic away from the macro into a function again that returns locks. There is still a macro to save repetition. I've also added back the metrics.

In the worst case, if it turns out that we really can't use the macro with async ops, we can always remove it again: now it's pretty cheap to do so (there is still some minor level of repetition that it saves).

[koivunej]

Adding request changes for this thread at least: https://github.com/neondatabase/neon/pull/5247/files#r1321117325

I still have issues with the tt-muncher and lack of Arc (unless I missed it) but now the metrics are back, and this needs to be rebased.

[arpad-m]

I still have issues with the tt-muncher and lack of Arc (unless I missed it) but now the metrics are back, and this needs to be rebased.

regarding the tt muncher I have now implemented your suggestion to not use the dot syntax any more. For the lack of Arc, it's not really needed to use Arcs here, instead I used lock objects as suggested by Heikki (already done that at the time you made that comment). For the rebase, I've done it via a git merge.

[koivunej]

Ha, it worked! I'm okay with this now, even if the Arc changes might be still required. Also drop spawns a task, it should be good.