pg-extend-rs
pg-extend-rs copied to clipboard
bluejekyll
Support shared_preload_libraries etc...
Hi,
Just wanted to know how you'd feel about adding support for shared_preload_libraries / bgworkers \ shared memory (although that's a lot harder) being added?
I’m a big fan of us using this library for extending Postgres in as many different ways as possible.
I’m currently a little concerned about the memory model for a lot of this, but yes, I’m a big supporter.
What are the concerns? I see you’re using palloc in behind a feature gate - which is neat. I also saw a comment about state being lost - but that’s what shared memory is for.
Admittedly writing to Shared Memory is a bit iffy in my experience - that can be worked on though.
On Wed, 22 May 2019 at 8:13 pm, Benjamin Fry [email protected] wrote:
I’m a big fan of us using this library for extending Postgres in as many different ways as possible.
I’m currently a little concerned about the memory model for a lot of this, but yes, I’m a big supporter.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bluejekyll/pg-extend-rs/issues/45?email_source=notifications&email_token=AAJJDI5HMMJ6TM7FZXW7MCLPWUMFRA5CNFSM4HORMQD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV6SRYQ#issuecomment-494741730, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJJDI7EY5ENB3YQPFT7WYTPWUMFRANCNFSM4HORMQDQ .
I’m working on a patch to try and make things threadsafe in regards to the use of palloc. I can reproduce issues with it right now, but don’t have it isolated to what the exact problem is, yet.
I’m struggling to get my head round a usecase for threading without loading as a shared library?
Are you on Slack or irc anywhere?
On Thu, 23 May 2019 at 12:49 am, Benjamin Fry [email protected] wrote:
I’m working on a patch to try and make things threadsafe in regards to the use of palloc. I can reproduce issues with it right now, but don’t have it isolated to why the exact problem is, yet.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bluejekyll/pg-extend-rs/issues/45?email_source=notifications&email_token=AAJJDIYGM7NZ472MYDM7H3DPWVMQ7A5CNFSM4HORMQD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV7JVWI#issuecomment-494836441, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJJDIZWST3NM7QWFFGQAS3PWVMQ7ANCNFSM4HORMQDQ .
I hang out in the Rust discord app, we can chat there.
This is all in the context of the shared library loading, as currently implemented. Threading would be from the Rust, though it might be in appropriate in general in Postgres, but I expect people coming from Rust will want to use threading...
Just saw this issue I'd forgotten and thought I'd comment with the current state of play - turns out calling Postgres through FFI (as this crate does) is unsafe even in the presence of threads.
ie: if you use Rayon, or something which starts a thread to do work in the background then you are firmly into the realms of UB - even if the threads never call into Postgres.
I agree that threads with PG FFI are inherently dangerous, given the nature of PG.
I think an option we have is to pass around a & mut instance of the memory context, and that would not be sendable across threads. If we required that handle on all FFI, then it would be possible to reduce mistakes in that context.
Not sure it’s worth it though. BG workers are probably the safe and best model here.
The issue with that approach is usually just because something can’t be sent between threads doesn’t mean it can’t be created on another thread.
Bgworkers suffer the same fate, with the small proviso that they can thread if they never connect to SPI, call PG functions, or deal with signals.
On Sun, 30 Aug 2020 at 12:50 am, Benjamin Fry [email protected] wrote:
I agree that threads with PG FFI are inherently dangerous, given the nature of PG.
I think an option we have is to pass around a & mut instance of the memory context, and that would not be sendable across threads. If we required that handle on all FFI, then it would be possible to reduce mistakes in that context.
Not sure it’s worth it though. BG workers are probably the safe and best model here.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bluejekyll/pg-extend-rs/issues/45#issuecomment-683300823, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJJDI6KZLSSZG6BV5XC3G3SDEISXANCNFSM4HORMQDQ .
So we already mark PgDatum and PgAllocated types as both !Send and !Sync specifically to deny sharing memory managed by PG between threads. Additionally, the PgAllocator is !Send and !Sync.
Otherwise we use the default Rust allocator for all memory. Is there a reason you think that isn't safe? I'm not honestly sure we have any changes necessary with those restrictions in place.
The FFI calls are still dangerous, I agree about that though.
The handle on FFI idea is interesting - even things like postgresql logging from a thread are unsafe sadly.
On Mon, 31 Aug 2020 at 9:38 am, Benjamin Fry [email protected] wrote:
So we already mark PgDatum and PgAllocated types as both !Send and !Sync specifically to deny sharing memory managed by PG between threads.
Otherwise we use the default Rust allocator for all memory. Is there a reason you think that isn't safe? I'm not honestly sure we have any changes necessary with those restrictions in place.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bluejekyll/pg-extend-rs/issues/45#issuecomment-683486204, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJJDIZD5QJLMUL5TQBFA2TSDLPH3ANCNFSM4HORMQDQ .