RE: Fix inappropriate comments in function ReplicationSlotAcquire

From: "Wei Wang (Fujitsu)" <wangw(dot)fnst(at)fujitsu(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: Fix inappropriate comments in function ReplicationSlotAcquire
Date: 2024-01-25 10:30:55
Message-ID: OSZPR01MB627824E11BAD42443D453E0D9E7A2@OSZPR01MB6278.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 25, 2024 at 18:39 Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>

Thanks for your review.

> On Thu, Jan 25, 2024 at 2:57 PM Wei Wang (Fujitsu)
> <wangw(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > In the function ReplicationSlotAcquire(), I found there is a missing in the
> > below comments:
> >
> > ```
> > /*
> > * Search for the slot with the specified name if the slot to acquire is
> > * not given. If the slot is not found, we either return -1 or error out.
> > */
> > s = SearchNamedReplicationSlot(name, false);
> > if (s == NULL || !s->in_use)
> > {
> > LWLockRelease(ReplicationSlotControlLock);
> >
> > ereport(ERROR,
> > (errcode(ERRCODE_UNDEFINED_OBJECT),
> > errmsg("replication slot \"%s\" does not
> exist",
> > name)));
> > }
> > ```
> >
> > It seems that when the slot is not found, we will only error out and will not
> > return -1.
> >
>
> You seem to be correct. However, isn't the first part of the comment
> also slightly confusing? In particular, "... if the slot to acquire is
> not given." In this function, I don't see the case where a slot to
> acquire is given.

Yes, agree. I think these two parts have become slightly outdated after the
commit 1632ea4. So also tried to fix the first part of the comment.
Attach the new patch.

Regards,
Wang Wei

Attachment Content-Type Size
v2-0001-Fix-inappropriate-comments-in-function-Replicatio.patch application/octet-stream 1.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2024-01-25 10:45:31 Re: index prefetching
Previous Message Amit Kapila 2024-01-25 10:24:45 Re: Synchronizing slots from primary to standby