Re: pg_stat_have_stats() returns true for dropped indexes (or for index creation transaction rolled back) - Mailing list pgsql-hackers

From Drouvot, Bertrand
Subject Re: pg_stat_have_stats() returns true for dropped indexes (or for index creation transaction rolled back)
Date
Msg-id 3cd209a4-e2a1-0ae6-91a3-c945eb7db248@amazon.com
Whole thread Raw
In response to pg_stat_have_stats() returns true for dropped indexes (or for index creation transaction rolled back)  ("Drouvot, Bertrand" <bdrouvot@amazon.com>)
Responses Re: pg_stat_have_stats() returns true for dropped indexes (or for index creation transaction rolled back)
List pgsql-hackers

Hi,

On 8/31/22 9:26 PM, Andres Freund wrote:
Hiu,

On 2022-08-25 11:44:34 +0200, Drouvot, Bertrand wrote:
For REPLSLOT, I agree that we can add one test: I added it in
contrib/test_decoding/sql/stats.sql. It relies on pg_stat_have_stats() (as
relying on pg_stat_replication_slots and/or pg_stat_get_replication_slot()
would not help that much for this test given that the slot has been removed
from ReplicationSlotCtl)
As Horiguchi-san noted, we can't rely on specific indexes being used. 

Yeah.

I feel
ok with the current coverage in that area, but if we *really* feel we need to
test it, we'd need to count the number of indexes with slots before dropping
the slot and after the drop.

Thanks for the suggestion, I'm coming up with this proposal in v4 attached:

  • count the number of slots
  • ensure we have at least one for which pg_stat_have_stats() returns true
  • get the list of ids (true_ids) for which pg_stat_have_stats() returns true
  • drop all the slots
  • get the list of ids (false_ids) for which pg_stat_have_stats() returns false
  • ensure that both lists (true_ids and false_ids) are the same

I don't "really" feel the need we need to test it but i think that this thread was a good opportunity to try to test it.

That said, that's also fine for me if this test is not part of the patch.

Maybe a better/simpler option could be to expose a function to get the slot id based on its name and then write a "simple" test with it? (If so I think that would better to start another patch/thread).

+-- pg_stat_have_stats returns true for committed index creation
Maybe another test for an uncommitted index creation would be good too?

You mean in addition to the "-- pg_stat_have_stats returns false for rolled back index creation" one?

Could you try running this test with debug_discard_caches = 1 - it's pretty
easy to write tests in this area that aren't reliable timing wise.

Thanks for the suggestion. I did and it passed without any issues.

+CREATE table stats_test_tab1 as select generate_series(1,10) a;
+CREATE index stats_test_idx1 on stats_test_tab1(a);
+SELECT oid AS dboid from pg_database where datname = current_database() \gset
Since you introduced this, maybe convert the other instance of this query at
the end of the file as well?

yeah good point. In v4, I moved the dboid recording at the top and use it when appropriate.

Regards,
--

Bertrand Drouvot
Amazon Web Services: https://aws.amazon.com
Attachment

pgsql-hackers by date:

Previous
From: Junwang Zhao
Date:
Subject: Re: [PATCH v1] [doc] polish the comments of reloptions
Next
From: Pavel Stehule
Date:
Subject: broken table formatting in psql