Re: Rotten parts of src/backend/replication/README

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Rotten parts of src/backend/replication/README
Date: 2020-05-02 11:24:32
Message-ID: CAA4eK1LUtf=Y98s9-GruowgA2RMPTggaEwhZ8+fs1h0LtsqAhw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, May 2, 2020 at 8:16 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> Hi all,
>
> The first part of src/backend/replication/README lists all the APIs
> usable for a WAL receiver, but these have aged and lost track of most
> changes that happened over the years. Four functions are listed in
> the README, with incorrect signatures and many others are missing:
> - walrcv_connect()
> - walrcv_receive()
> - walrcv_send()
> - walrcv_disconnect()
>
> I think that we should clean up that.

+1.

> And as it seems to me that
> nobody really remembers to update this README, I would suggest to
> update the first section of the README to refer to walreceiver.h for
> details about each function, then move the existing API descriptions
> from the README to walreceiver.h (while fixing the incomplete
> descriptions of course). This way, if a new function is added or if
> an existing function is changed, it is going to be hard to miss an
> update of the function descriptions.
>

I think in README we can have a general description of the module and
maybe at the broad level how different APIs can help to achieve the
required functionality and then for API description we can refer to
.h/.c. The detailed description of APIs should be where those APIs
are defined. The header file can contain some generic description.
The detailed description I am referring to is below in the README:
"Retrieve any message available without blocking through the
connection. If a message was successfully read, returns its length.
If the connection is closed, returns -1. Otherwise returns 0 to
indicate that no data is available, and sets *wait_fd to a socket
descriptor which can be waited on before trying again. On success, a
pointer to the message payload is stored in *buffer. The returned
buffer is valid until the next call to walrcv_* functions, and the
caller should not attempt to free it."

I think having such a description near the actual definition helps in
keeping it updated whenever we change the function.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-05-02 11:40:38 Re: Back-patch is necessary? Re: Don't try fetching future segment of a TLI.
Previous Message Tomas Vondra 2020-05-02 10:55:00 Re: SLRU statistics