MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Thu, 01 Feb 2024 21:22:48 +0900
From: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Fix bugs not to discard statistics when changing
 stats_fetch_consistency
In-Reply-To: <ZbtXYVvdgRsUVVwA@paquier.xyz>
References: <d77fc5190d4dbe1738d77231488e768b@oss.nttdata.com>
 <ZbtXYVvdgRsUVVwA@paquier.xyz>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <fc1cce6f1c4f3f1f9bb60c29302ae140@oss.nttdata.com>
Archived-At: 
 <https://www.postgresql.org/message-id/fc1cce6f1c4f3f1f9bb60c29302ae140%40oss.nttdata.com>
Precedence: bulk

On 2024-02-01 17:33, Michael Paquier wrote:
> On Thu, Jan 11, 2024 at 06:18:38PM +0900, Shinya Kato wrote:
>> Hi, hackers
> 
> (Sorry for the delay, this thread was on my TODO list for some time.)
>> There is below description in docs for stats_fetch_consistency.
>> "Changing this parameter in a transaction discards the statistics 
>> snapshot."
>> 
>> However, I wonder if changes stats_fetch_consistency in a transaction,
>> statistics is not discarded in some cases.
> 
> Yep, you're right.  This is inconsistent with the documentation where
> we need to clean up the cached data when changing this GUC.  I was
> considering a few options regarding the location of the extra
> pgstat_clear_snapshot(), but at the end I see the point in doing it in
> a path even if it creates a duplicate with pgstat_build_snapshot()
> when pgstat_fetch_consistency is using the snapshot mode.  A location
> better than your patch is pgstat_snapshot_fixed(), though, so as new
> stats kinds will be able to get the call.
> 
> I have been banging my head on my desk for a bit when thinking about a
> way to test that in a predictible way, until I remembered that these
> stats are only flushed at commit, so this requires at least two
> sessions, with one of them having a transaction opened while
> manipulating stats_fetch_consistency.  TAP would be one option, but
> I'm not really tempted about spending more cycles with a
> background_psql just for this case.  If I'm missing something, feel
> free.

Thank you for the review and pushing!
I think it is better and more concise than my implementation.

-- 
Regards,
Shinya Kato
NTT DATA GROUP CORPORATION