| 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: | Whole Thread | Raw Message | 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 |
| 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 |