Re: [HACKERS] Restricting maximum keep segments by repslots

Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>

From: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
To: sawada.mshk@gmail.com
Cc: pgsql-hackers@lists.postgresql.org, thomas.munro@enterprisedb.com, sk@zsrv.org, michael.paquier@gmail.com, andres@anarazel.de, peter.eisentraut@2ndquadrant.com
Date: 2018-09-04T10:52:50Z
Lists: pgsql-hackers

Attachments

At Mon, 3 Sep 2018 18:14:22 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in <CAD21AoBgCMc9bp2cADMFm40qoEXxbomdu1dtj5EaFSAS4BtAyw@mail.gmail.com>
> Thank you for updating the patch!
> 
> On Tue, Jul 31, 2018 at 6:11 PM, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> > Hello.
> >
> > At Tue, 24 Jul 2018 16:47:41 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in <CAD21AoD0rChq7wQE=_o95quopcQGjcVG9omwdH07nT5cm81hzg@mail.gmail.com>
> >> On Mon, Jul 23, 2018 at 4:16 PM, Kyotaro HORIGUCHI
> >> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> >> > Hello.
> >> >
> >> > At Fri, 20 Jul 2018 10:13:58 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in <CAD21AoDayePWwu4t=VPP5P1QFDSBvks1d8j76bXp5rbXoPbZcA@mail.gmail.com>
> >> This funk should be updated.
> >
> > Perhaps you need a fresh database cluster.
> 
> I meant this was a doc update in 0004 patch but it's fixed in v7 patch.

Wow..

> While testing the v7 patch, I got the following result with
> max_slot_wal_keep_size = 5GB and without wal_keep_segments setting.
> 
> =# select pg_current_wal_lsn(), slot_name, restart_lsn,
> confirmed_flush_lsn, wal_status, remain, pg_size_pretty(remain) from
> pg_replication_slots ;
>  pg_current_wal_lsn | slot_name | restart_lsn | confirmed_flush_lsn |
> wal_status |  remain  | pg_size_pretty
> --------------------+-----------+-------------+---------------------+------------+----------+----------------
>  2/A30000D8         | l1        | 1/AC000910  | 1/AC000948          |
> streaming  | 16777000 | 16 MB
> (1 row)
> 
> The actual distance between the slot limit and the slot 'l1' is about
> 1GB(5GB - (2/A30000D8 - 1/AC000910)) but the system view says the
> remain is only 16MB. For the calculation of resetBytes in
> GetOldestKeepSegment(), the current patch seems to calculate the
> distance between the minSlotLSN and restartLSN when (curLSN -
> max_slot_wal_keep_size) < minSlotLSN. However, I think that the actual
> remained bytes until the slot lost the required WAL is (restartLSN -
> (currLSN - max_slot_wal_keep_size)) in that case.

Oops! That's a silly thinko or rather a typo. It's apparently
wrong that keepSegs instead of limitSegs is involved in making
the calculation of restBytes. Just using limitSegs makes it
sane. It's a pity that I removed the remain from regression test.

Fixed that and added an item for remain calculation in the TAP
test. I expect that pg_size_pretty() adds some robustness to the
test.

> Also, 0004 patch needs to be rebased on the current HEAD.

Done. Please find the v8 attached.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Commits

  1. Save slot's restart_lsn when invalidated due to size

  2. Fix checkpoint signalling

  3. Check slot->restart_lsn validity in a few more places

  4. Allow users to limit storage reserved by replication slots

  5. Remove header noise from test_decoding test

  6. Rework WAL-reading supporting structs

  7. Flip argument order in XLogSegNoOffsetToRecPtr