RE: [Patch] Optimize dropping of relation buffers using dlist

From: "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>
To: 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>, "amit(dot)kapila16(at)gmail(dot)com" <amit(dot)kapila16(at)gmail(dot)com>, "k(dot)jamison(at)fujitsu(dot)com" <k(dot)jamison(at)fujitsu(dot)com>
Cc: "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, "tgl(at)sss(dot)pgh(dot)pa(dot)us" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "thomas(dot)munro(at)gmail(dot)com" <thomas(dot)munro(at)gmail(dot)com>, "robertmhaas(at)gmail(dot)com" <robertmhaas(at)gmail(dot)com>, "tomas(dot)vondra(at)2ndquadrant(dot)com" <tomas(dot)vondra(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: [Patch] Optimize dropping of relation buffers using dlist
Date: 2020-12-09 01:57:42
Message-ID: TYAPR01MB2990EE732D5399DEC6FFD2F3FECC0@TYAPR01MB2990.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
> At Tue, 8 Dec 2020 16:28:41 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote in
> I also can't think of a way to use an optimized path for such cases
> > but I don't agree with your comment on if it is common enough that we
> > leave this optimization entirely for the truncate path.
>
> An ugly way to cope with it would be to let other smgr functions
> manage the cached value, for example, by calling smgrnblocks while
> InRecovery. Or letting smgr remember the maximum block number ever
> accessed. But we cannot fully rely on that since smgr can be closed
> midst of a session and smgr doesn't offer such persistence. In the
> first place smgr doesn't seem to be the place to store such persistent
> information.

Yeah, considering the future evolution of this patch to operations during normal running, I don't think that would be a good fit, either.

Then, the as we're currently targeting just recovery, the options we can take are below. Which would vote for? My choice would be (3) > (2) > (1).

(1)
Use the cached flag in both VACUUM (0003) and TRUNCATE (0004).
This brings the most safety and code consistency.
But this would not benefit from optimization for TRUNCATE in unexpectedly many cases -- when TOAST storage exists but it's not written, or FSM/VM is not updated after checkpoint.

(2)
Use the cached flag in VACUUM (0003), but use InRecovery instead of the cached flag in TRUNCATE (0004).
This benefits from the optimization in all cases.
But this lacks code consistency.
You may be afraid of safety if the startup process smgrclose()s the relation after the shared buffer flushing hits disk full. However, startup process doesn't smgrclose(), so it should be safe. Just in case the startup process smgrclose()s, the worst consequence is PANIC shutdown after repeated failure of checkpoints due to lingering orphaned dirty shared buffers. Accept it as Thomas-san's devil's suggestion.

(3)
Do not use the cached flag in either VACUUM (0003) or TRUNCATE (0004).
This benefits from the optimization in all cases.
The code is consistent and smaller.
As for the safety, this is the same as (2), but it applies to VACUUM as well.

Regards
Takayuki Tsunakawa

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message k.jamison@fujitsu.com 2020-12-09 02:08:27 RE: [Patch] Optimize dropping of relation buffers using dlist
Previous Message Tom Lane 2020-12-09 01:38:27 Re: [HACKERS] [PATCH] Generic type subscripting