Re: Fwd: dsm_registry: Add detach and destroy features

Hi,

>
> Thanks for the patch. I'm still interested to hear whether the proposed
> GetNamedDSA() feature could work for your use-case, as I described upthread
> [0].
>
> [0] https://postgr.es/m/aFAszMKD69AyA4c1%40nathan

Is this improvement still being worked on? I think it will be a useful
addition to the dsm_registry,
if we could delete the entries from the dsm_registry_table after they
are no longer used by any process.

> Right, you'd need some other shared space for the DSA pointers. In the
> other thread, I'm using a dshash table (created via GetNamedDSMHash()) to
> store those for test_dsm_registry [0]. There are probably other ways to do
> this.

It would be nice if we could store the dsa_pointers generated by
dsa_allocate calls
in the dsm_registry itself, this way each module using dsa won't need
to define its
own table for storing the pointers. I am not yet sure how it will fit
in the existing structure
of the registry table, though.

Please find a few high level comments on the v4 patch below.

> +typedef struct DSMDetachCallbackContext
> +{
> + void (*on_detach_callback) (void *);
> + void *arg;
> +} DSMDetachCallbackContext;
> +

I am wondering why we need to add the ability to define on_dsm_detach
callbacks in the dsm_registry.
Isn't the existing on_dsm_detach functionality sufficient?

> +extern void DetachNamedDSMSegment(const char *name, size_t size,
> + void (*on_detach_callback) (void *),
> + void *arg);
> +
> +extern void DestroyNamedDSMSegment(const char *name, size_t size,
> + void (*on_detach_callback) (void *),
> + void *arg);

Why do we need two separate functions? Since DestroyNamedDSMSegment()
also ensures
that the segment is detached, having just one function seems sufficient.
For cases where we only need to detach the segments, can't we use
dsm_detach() directly?

Thank you,
Rahila Syed