Re: Failed Assert while pgstat_unlink_relation

Hi,

On 2022-11-29 15:42:45 +0900, Kyotaro Horiguchi wrote:
> At Mon, 28 Nov 2022 14:01:30 +0530, vignesh C <vignesh21(at)gmail(dot)com> wrote in
> > Hi,
> >
> > While reviewing/testing one of the patches I found the following Assert:
> > #0 0x000055c6312ba639 in pgstat_unlink_relation (rel=0x7fb73bcbac58)
> > at pgstat_relation.c:161
> > #1 0x000055c6312bbb5a in pgstat_relation_delete_pending_cb
> > (entry_ref=0x55c6335563a8) at pgstat_relation.c:839
> > #2 0x000055c6312b72bc in pgstat_delete_pending_entry
> > (entry_ref=0x55c6335563a8) at pgstat.c:1124
> > #3 0x000055c6312be3f1 in pgstat_release_entry_ref (key=...,
> > entry_ref=0x55c6335563a8, discard_pending=true) at pgstat_shmem.c:523
> > #4 0x000055c6312bee9a in pgstat_drop_entry
> > (kind=PGSTAT_KIND_RELATION, dboid=5, objoid=40960) at
> > pgstat_shmem.c:867
> > #5 0x000055c6312c034a in AtEOXact_PgStat_DroppedStats
> > (xact_state=0x55c6334baac8, isCommit=false) at pgstat_xact.c:97
> > #6 0x000055c6312c0240 in AtEOXact_PgStat (isCommit=false,
> > parallel=false) at pgstat_xact.c:55
> > #7 0x000055c630df8bee in AbortTransaction () at xact.c:2861
> > #8 0x000055c630df94fd in AbortCurrentTransaction () at xact.c:3352
> >
> > I could reproduce this issue with the following steps:
> > create table t1(c1 int);
> > BEGIN;
> > CREATE TABLE t (c1 int);
> > CREATE RULE "_RETURN" AS ON SELECT TO t DO INSTEAD SELECT * FROM t1;
> > CREATE RULE "_RETURN" AS ON SELECT TO t DO INSTEAD SELECT * FROM t1;
>
> Good catch!
>
> AtEOXact_PgStat_DroppedStats() visits a relation that has been dropped
> then wiped (when CLOBBER_FREED_MEMORY) by AtEOXact_RelationCache()
> called just before. Since the relcache content directly pointed from
> stats module is lost in this case, the stats side cannot defend itself
> from this.
>
> Perhaps RelationDestroyRelation() need to do pgstat_drop_entry() or
> AtEOXact_PgStat_DroppedStats() should be called before
> AtEOXact_RelationCache(). the latter of which is simpler. I think we
> need to test this case, too.

This doesn't strike me as the right fix. What do you think about my patch at
https://postgr.es/m/20221128210908.hyffmljjylj447nu%40awork3.anarazel.de ,
leaving the quibbles around error handling aside?

Afaict it fixes the issue.

Greetings,

Andres Freund