Dear Amit, Shveta,
Thanks for suggestions. PSA new patches.
> > Since user is not aware of internal acquired_by logic, the error might
> > not make much sense to him as to why one session is able to reset
> > while another is not. Shall we make it:
> >
> > ERROR: cannot reset replication origin "origin_name" while it is
> > still shared by other processes
> > DETAIL: the current session is the first process for this replication
> > origin, and other processes are sharing it.
> > HINT: ensure this replication origin is reset in all other processes first.
> >
>
> How about a slightly tweaked version of these messages:
> ERROR: cannot reset replication origin "origin_name" because it is
> still in use by other processes
> DETAIL: This session is the first process for this replication origin,
> and other processes are currently sharing it.
> HINT: Reset the replication origin in all other processes before retrying.
I followed the Amit's idea, but the origin ID is used instead of origin name.
I read other functions, and the name is directly output when 1) the specified
origin does not exist or 2) the name is reserved. We seem to use ID as much as
possible.
>
> > 2)
> > When the first session leaves, while the second session is still using
> > origin, the third session is able to drop the origin which is not
> > right.
> > I think replorigin_state_clear() needs a change.
> > 'if (state->acquired_by != 0)' check should be replaced by 'if
> > (state->refcount > 0)'
> >
> > Patch 001 had correct changes in replorigin_state_clear(), IMO we
> > still need those
Good finding. I put it in 0002 because it handles some cases related with
acquired_by = 0.
> >
> > 3)
> > When first session leaves, while second session is still using origin,
> > now correctly third session is not able to join it. It gives error:
> > postgres=# SELECT pg_replication_origin_session_setup('origin');
> > ERROR: replication origin with ID 1 is already active for another process
> >
> > Error is not very informative provided the fact that now sharing is
> > allowed. Shall it be:
> >
>
> Yeah, sharing is allowed but only when used in parallel context by
> passing PID. I think a slightly modified version of the above message
> such as: "replication origin with ID 1 is already active in another
> process" should be sufficient.
Fixed but ereport() was used because I thought this is usar-facing. Feel free to
change to elog() again based on your matter.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
