pgcentralfoundation / pgrx

Build Postgres Extensions with Rust!
Other
3.57k stars 242 forks source link

Design discussion for async/await support #1067

Open JelteF opened 1 year ago

JelteF commented 1 year ago

I'm interested in having good support for async/await in pgx. So I wanted to share some of the thoughts I had on what this would look like.

First of all, let's note what's currently already possible. Because this simply already works:

#[pg_extern]
#[tokio::main(flavor = "current_thread")]
async fn get_ip() -> Result<String, reqwest::Error> {
    reqwest::get("https://api.ipify.org").await?.text().await
}

You can then call SELECT get_ip() and everything seems good.

However, there's two problems that I see with this approach:

  1. A new tokio Runtime is created for each call of this function. It's debatable how much of a problem this is, but preferably there would only be one runtime.
  2. While awaiting no interrupts are being handled. It would be best if the tokio Runtime would also poll check_for_interupts!() once in a while.

I have some ideas on how to solve number 2, but number 1 seems more difficult

eeeebbbbrrrr commented 1 year ago

I'm sure this will be an interesting discussion! We're all kinda heads-down here in pgxland right now, but we'll get to responding to this sometime soon.

I personally don't have enough experience with Rust async to offer much, but I suspect there's an entire list of things we'll need to address around async runtime + postgres interactions.

workingjubilee commented 1 year ago

The big issue is going to be making sure that a tokio runtime under PGX doesn't cause off-main multithreaded interactions with Postgres.

JelteF commented 1 year ago

The big issue is going to be making sure that a tokio runtime under PGX doesn't cause off-main multithreaded interactions with Postgres.

Afaict that problem is solved by using the current_thread flavor:

#[tokio::main(flavor = "current_thread")]
workingjubilee commented 1 year ago

I haven't used tokio a lot but we definitely can't use tokio::main for dylibs that don't even have a main function, so that's gonna have to come from somewhere else.

JelteF commented 1 year ago

Based on the tokio docs it seems acceptable, but not preferable:

Note: This macro can be used on any function and not just the main function. Using it on a non-main function makes the function behave as if it was synchronous by starting a new runtime each time it is called. If the function is called often, it is preferable to create the runtime using the runtime builder so the runtime can be reused across calls.

https://docs.rs/tokio/latest/tokio/attr.main.html

workingjubilee commented 1 year ago

Then we should probably properly use a Builder.

einarmo commented 1 year ago

I have some experience with this, though perhaps not very encouraging:

I did this for a foreign data wrapper which relied on some async libraries. I used a lazy static tokio runtime.

It may be possible to force it to run on postgres' thread, but I never managed that (even using current_thread?, it's been a while, I might be wrong), and I wanted multi-threading anyway. Any attempt at running postgres functions within the tokio runtime would instantly segfault, so I essentially convert anything from postgres into rust-allocated data structures and use those within threading. In the end that was not really any additional burden, since you generally don't want to juggle postgres structs full of raw pointers in your safe code anyway.

In the code it was as simple to use as,

TOKIO_RUNTIME.block_on(async { });

which you could easily create a #[pg_extern_async] or something macro for, if there was a good way to avoid the massive soundness issue of not moving out of the current thread.

skyzh commented 1 year ago

I feel an easier way to do that is to spawn some tokio threads on PG_INIT and use channels / tokio::runtime::Runtime::spawn to dispatch jobs to tokio runtime upon entering async functions...