> As I did not hear back, I went ahead and prepared a patch with the above.
If the question was for me sorry for not replying, I assumed it was
meant for Nathan.
Personally I'm not sure if we need this as even requiring a name isn't
a common use case, but I'm also fine with this version.
The only additional thing I would do is to add some kind of test that
verifies that we indeed forward the pointer to the callback in
test_dsa, for example:
+const char* forwardedData = "ForwardedData";
+
static void
-init_tranche(void *ptr)
+init_tranche(void *ptr, const char *name, void *arg)
{
int *tranche_id = (int *) ptr;
- *tranche_id = LWLockNewTrancheId("test_dsa");
+ if (arg != forwardedData) {
+ elog(ERROR, "Didn't receive expected arg pointer");
+ }
+
+ *tranche_id = LWLockNewTrancheId(name);
}
As otherwise we no longer test the value of the pointer in any of the
tests with the additional name parameter.
On a slightly related topic, I mentioned earlier that I found 3
extensions using GetNamedDSMSegment. One of them,
pg_track_optimizer[1] could use the new GetNamedDSHash function,
except that it doesn't provide a callback similar to what we have in
GetNamedDSMSegment, and it relies on initializing the hash in the
callback, before it returns. That's not possible with the new
function.
What do you think, would it make sense to also include callbacks for
GetNamedDSHash and GetNamedDSA?
[1]: https://github.com/danolivo/pg_track_optimizer/
On Thu, Dec 11, 2025 at 10:12 PM Sami Imseih <[email protected]> wrote:
>
> > This works well for the first use-case identified. Instead of hard-
> > coding the tranche name in the callback, the name can be
> > retrieved as the segment name set in GetNamedDSMSegment.
> >
> > The caller could still pass this name via the extra callback args, but
> > it's better to separate things a bit here, and reserve the extra callback
> > arguments for more complex data.
> >
> > What do you think?
>
> As I did not hear back, I went ahead and prepared a patch with the above.
>
> I went back-forth on if it makes sense to provide the name as an
> extra argument and decided it provides more flexibility. For example
> I can use the same init callback and arguments for different segments.
>
> Also, the name provides a guarantee of the name of the segment that
> this callback is initializing.
>
> Overall, I felt it was a better approach.
>
> I updated the code comments and documentation.
>
> Also, in test_dsa.c, i updated the name of the segments to
> reflect the name of the functions creating the segments, like
> below:
> ```
> @@ -38,8 +38,8 @@ test_dsa_basic(PG_FUNCTION_ARGS)
> dsa_area *a;
> dsa_pointer p[100];
>
> - tranche_id = GetNamedDSMSegment("test_dsa", sizeof(int),
> -
> init_tranche, &found);
> + tranche_id = GetNamedDSMSegment("test_dsa_basic", sizeof(int),
> +
> init_tranche, NULL, &found);
>
> @@ -79,8 +79,8 @@ test_dsa_resowners(PG_FUNCTION_ARGS)
> ResourceOwner oldowner;
> ResourceOwner childowner;
>
> - tranche_id = GetNamedDSMSegment("test_dsa", sizeof(int),
> -
> init_tranche, &found);
> + tranche_id = GetNamedDSMSegment("test_dsa_resowners", sizeof(int),
> +
> init_tranche, NULL, &found);
>
>
> ```
> This is good for showing the same init callback being re-used
> for different named segment initizations.
>
> I also updated the commit message.
>
> --
> Sami Imseih
> Amazon Web Services (AWS)
v3-0001-Pass-name-and-callback-data-to-GetNamedDSMSegment-in.patch
Description: Binary data
