Re: Ideas for a relcache test mode about missing invalidations - Mailing list pgsql-hackers

Hello.

At Wed, 1 Aug 2018 09:25:18 -0700, Andres Freund <andres@anarazel.de> wrote in
<20180801162518.jnb2ql5dfmgwp4qo@alap3.anarazel.de>
> Hi,
> 
> The issue at [1] is caused by missing invalidations, and [2] seems like
> a likely candidate too. I wonder if it'd be good to have a relcache test
> mode akin to CLOBBER_CACHE_ALWAYS and RELCACHE_FORCE_RELEASE, that tries
> to ensure that we've done sufficiently to ensure the right invalidations
> are sent.
> 
> I think what we'd kind of want is to ensure that relcache entries are
> rebuilt at the earliest possible time, but *not* later. That'd mean
> they're out of date if there's missing invalidations. Unfortunately I'm
> not clear on how that'd be achievable?  Ideas?
> 
> The best I can come up with is to code some additional dependencies into
> CacheInvalidateHeapTuple(), and add tracking ensuring we've sent the
> right messages. But that seems somewhat painful and filled with holes.
> 
> [1]
http://archives.postgresql.org/message-id/CAKoxK%2B5fVodiCtMsXKV_1YAKXbzwSfp7DgDqUmcUAzeAhf%3DHEQ%40mail.gmail.com
> [2] https://www.postgresql.org/message-id/12259.1532117714@sss.pgh.pa.us

As for [1], it is not a issue on invalidation. It happens also if
the relation has any index and even drop is not needed. The
following steps are sufficient.

create table t( pk serial, t text );
insert into t( t ) values( 'hello' ), ('world');
create index idx_fake0 on t (pk);
create index idx_fake on t ( f_fake( pk ) ); -- ERROR

index_create() creates a new pg_index entry with indislive = true
before building it. So the planner (RelationGetIndexList) sees
the not-yet-populated index while planning the query in f_fake().

The attached patch fixes the issue, but I'm not sure this story
is applicable to [2].

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From eaa5de68ff27bd43a643089f8c5963e5cc3d20cc Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 2 Aug 2018 18:52:24 +0900
Subject: [PATCH] Don't use currently-building index to populate it

index_create() creates a pg_index entry with indislive = true before
populating it. If it is a function index where the function runs a
query on the parent relation, planner sees the just created entry and
tries to access the heap page that is not created yet.  This patch let
index_create() not to set indislive = true after population.
---
 src/backend/catalog/index.c | 55 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 8b276bc430..b561c8696d 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -124,6 +124,7 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
                     bool immediate,
                     bool isvalid,
                     bool isready);
+static void ActivateIndexRelation(Oid indexoid);
 static void index_update_stats(Relation rel,
                    bool hasindex,
                    double reltuples);
@@ -666,7 +667,7 @@ UpdateIndexRelation(Oid indexoid,
     values[Anum_pg_index_indisvalid - 1] = BoolGetDatum(isvalid);
     values[Anum_pg_index_indcheckxmin - 1] = BoolGetDatum(false);
     values[Anum_pg_index_indisready - 1] = BoolGetDatum(isready);
-    values[Anum_pg_index_indislive - 1] = BoolGetDatum(true);
+    values[Anum_pg_index_indislive - 1] = BoolGetDatum(false);
     values[Anum_pg_index_indisreplident - 1] = BoolGetDatum(false);
     values[Anum_pg_index_indkey - 1] = PointerGetDatum(indkey);
     values[Anum_pg_index_indcollation - 1] = PointerGetDatum(indcollation);
@@ -693,6 +694,55 @@ UpdateIndexRelation(Oid indexoid,
     heap_freetuple(tuple);
 }
 
+/* ----------------------------------------------------------------
+ *        ActivateIndexRelation
+ *
+ * Publish index by marking it "relislive"
+
+ *  UpdateIndexRelation builds an index relation with relislive = false so as
+ *  not to be used by the quieries used to build the index. This function
+ *  marks the index as "live" so that it can be used hereafter.
+ *  ----------------------------------------------------------------
+ */
+static void
+ActivateIndexRelation(Oid indexoid)
+{
+    Relation    indrel;
+    SysScanDesc    sdesc;
+    ScanKeyData    skey;
+    HeapTuple    tup;
+
+    ScanKeyInit(&skey,
+                Anum_pg_index_indexrelid,
+                BTEqualStrategyNumber, F_OIDEQ,
+                ObjectIdGetDatum(indexoid));
+    indrel = heap_open(IndexRelationId, RowExclusiveLock);
+    sdesc = systable_beginscan(indrel, InvalidOid, true, NULL, 1, &skey);
+
+    /*
+     * We must see one and only one entry for the key. But don't bother
+     * checking that.
+     */
+    while (HeapTupleIsValid(tup = systable_getnext(sdesc)))
+    {
+        Datum values[Natts_pg_index];
+        bool nulls[Natts_pg_index];
+        bool replaces[Natts_pg_index];
+
+        MemSet(values, 0, sizeof(values));
+        MemSet(nulls, 0, sizeof(nulls));
+        MemSet(replaces, 0, sizeof(replaces));
+        values[Anum_pg_index_indislive] = BoolGetDatum(true);
+        replaces[Anum_pg_index_indislive] = true;
+        tup = heap_modify_tuple(tup, RelationGetDescr(indrel),
+                                values, nulls, replaces);
+        CatalogTupleUpdate(indrel, &tup->t_self, tup);
+        heap_freetuple(tup);
+    }
+    systable_endscan(sdesc);
+    heap_close(indrel, RowExclusiveLock);
+}
+
 
 /*
  * index_create
@@ -1188,6 +1238,9 @@ index_create(Relation heapRelation,
                     true);
     }
 
+    /* let queries use this index */
+    ActivateIndexRelation(indexRelationId);
+
     /*
      * Close the index; but we keep the lock that we acquired above until end
      * of transaction.  Closing the heap is caller's responsibility.
-- 
2.16.3