Re: Review for GetWALAvailability()

At Tue, 23 Jun 2020 19:06:25 -0400, Alvaro Herrera <alvherre@2ndquadrant.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