Re: Review for GetWALAvailability()

At Tue, 23 Jun 2020 19:06:25 -0400, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote in
> On 2020-Jun-16, Kyotaro Horiguchi wrote:
>
> > I saw that the "reserved" is the state where slots are working to
> > retain segments, and "normal" is the state to indicate that "WAL
> > segments are within max_wal_size", which is orthogonal to the notion
> > of "reserved". So it seems to me useless when the retained WAL
> > segments cannot exceeds max_wal_size.
> >
> > With longer description they would be:
> >
> > "reserved under max_wal_size"
> > "reserved over max_wal_size"
> > "lost some segements"
>
> > Come to think of that, I realized that my trouble was just the
> > wording. Are the following wordings make sense to you?
> >
> > "reserved" - retained within max_wal_size
> > "extended" - retained over max_wal_size
> > "lost" - lost some segments
>
> So let's add Unreserved to denote the state that it's over the slot size
> but no segments have been removed yet:

Oh! Thanks for the more proper word. It looks good to me.

> * Reserved under max_wal_size
> * Extended past max_wal_size, but still within wal_keep_segments or
> maximum slot size.
> * Unreserved Past wal_keep_segments and the maximum slot size, but
> not yet removed. Recoverable condition.
> * Lost lost segments. Unrecoverable condition.

Look good, too.

> It seems better to me to save the invalidation LSN in the persistent
> data rather than the in-memory data that's lost on restart. As is, we
> would lose the status in a restart, which doesn't seem good to me. It's
> just eight extra bytes to write ... should be pretty much free.

Agreed.

> This version I propose is based on the one you posted earlier today and
> is what I propose for commit.

- /* slot does not reserve WAL. Either deactivated, or has never been active */
+ /*
+ * slot does not reserve WAL. Either deactivated, or has never been active
+ */

Sorry, this is my fault. The change is useless. The code for
WALAVAIL_REMOVED looks good.

# Advance WAL again without checkpoint, reducing remain by 6 MB.
+$result = $node_master->safe_psql('postgres',
+ "SELECT wal_status, restart_lsn, min_safe_lsn FROM pg_replication_slots WHERE slot_name = 'rep1'"
+);
+print $result, "\n";

Sorry this is my fault, too. Removed in the attached.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center