(moving to pgsql-hackers)
On 29/01/2024 14:06, Heikki Linnakangas wrote:
> If you call dsa_allocate_extended(DSA_ALLOC_NO_OOM), it will still
> ereport an error if you run out of space (originally reported at [0]).
>
> Attached patch adds code to test_dsa.c to demonstrate that:
>
> postgres=# select test_dsa_basic();
> ERROR: could not resize shared memory segment "/PostgreSQL.1312700148"
> to 1075843072 bytes: No space left on device
>
> [0] https://github.com/pgvector/pgvector/issues/434#issuecomment-1912744489
I wrote the attached patch to address this, in a fairly straightforward
or naive way. The problem was that even though dsa_allocate() had code
to check the return value of dsm_create(), and return NULL instead of
ereport(ERROR) if the DSA_ALLOC_NO_OOM was set, dsm_create() does not in
fact return NULL on OOM. To fix, I added a DSM_CREATE_NO_OOM option to
dsm_create(), and I also had to punch that through to dsm_impl_op().
This is a little unpolished, but if we want to backpatch something
narrow now, this would probably be the right approach.
However, I must say that the dsm_impl_op() interface is absolutely
insane. It's like someone looked at ioctl() and thought, "hey that's a
great idea!". It mixes all different operations like creating or
destroying a DSM segment together into one function call, and the return
value is just a boolean, even though the function could fail for many
different reasons, and the callers do in fact care about the reason. In
a more natural interface, the different operations would have very
different function signatures.
I think we must refactor that. It might be best to leave this
DSA_ALLOC_NO_OOM bug unfixed in backpatches, and fix it on top of the
refactorings on master only. Later, we can backpatch the refactorings
too if we're comfortable with it; extensions shouldn't be using the
dsm_impl_op() interface directly.
(I skimmed through the thread where the DSM code was added, but didn't
see any mention of why dsm_impl_op() signature is like that:
https://www.postgresql.org/message-id/CA%2BTgmoaDqDUgt%3D4Zs_QPOnBt%3DEstEaVNP%2B5t%2Bm%3DFPNWshiPR3A%40mail.gmail.com)
--
Heikki Linnakangas
Neon (https://neon.tech)
