Re: Fetching timeline during recovery

On Mon, 9 Sep 2019 19:44:10 +0900
Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> On Sat, Sep 7, 2019 at 12:06 AM Jehan-Guillaume de Rorthais
> <jgdr(at)dalibo(dot)com> wrote:
> >
> > On Wed, 4 Sep 2019 00:32:03 +0900
> > Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> >
[...]
> Thanks for updating the patch!

Thank you for your review!

Please find in attachment a new version of the patch.

0001-v5-Add-facilities-to-fetch-real-timeline-from-SQL.patch

> Should we add regression tests for these functions? For example,
> what about using these functions to check the timeline switch case,
> in src/test/recovery/t/004_timeline_switch.pl?

Indeed, I added 6 tests to this file.

> [...]

Thank you for all other suggestions. They all make sense for v4 of the patch.
However, I removed pg_current_wal_tl() and pg_last_wal_received_tl() to explore
a patch paying attention to your next comment.

> I'm just imaging that some users want to use pg_last_wal_receive_lsn() and
> pg_last_wal_receive_tli() together to, e.g., get the name of WAL file received
> last. But there can be a corner case where the return values of
> pg_last_wal_receive_lsn() and of pg_last_wal_receive_tli() are inconsistent.
> This can happen because those values are NOT gotten within single lock.
> That is, each function takes each lock to get each value.
>
> So, to avoid that corner case and get consistent WAL file name,
> we might want to have the function that gets both LSN and
> timeline ID of the last received WAL record within single lock
> (i.e., just uses GetWalRcvWriteRecPtr()) and returns them.
> Thought?

You are right.

SO either I add some new functions or I overload the existing ones.

I was not convinced to add two new functions very close to pg_current_wal_lsn
and pg_last_wal_receive_lsn but with a slightly different name (eg. suffixed
with _tli?).

I choose to overload pg_current_wal_lsn and pg_last_wal_receive_lsn with
pg_current_wal_lsn(with_tli bool) and pg_last_wal_receive_lsn(with_tli bool).

Both function returns the record (lsn pg_lsn,timeline int4). If with_tli is
NULL or false, the timeline field is NULL.

Documentation is updated to reflect this.

Thoughts?

If this solution is accepted, some other function of the same family might be
good candidates as well, for the sake of homogeneity:

* pg_current_wal_insert_lsn
* pg_current_wal_flush_lsn
* pg_last_wal_replay_lsn

However, I'm not sure how useful this would be.

Thanks again for your time, suggestions and review!

Regards,