Re: [PATCH]: Not to invaldiate CatalogSnapshot for local invalidation messages

Hi
---setup.
drop table s2;
create table s2(a int);

After apply the patch
alter table s2 add primary key (a);

watch CatalogSnapshot
----
#0  GetNonHistoricCatalogSnapshot (relid=1259)
    at ../../Desktop/pg_src/src7/postgresql/src/backend/utils/time/snapmgr.c:412
#1  0x000055ba78f0d6ba in GetCatalogSnapshot (relid=1259)
    at ../../Desktop/pg_src/src7/postgresql/src/backend/utils/time/snapmgr.c:371
#2  0x000055ba785ffbe1 in systable_beginscan
(heapRelation=0x7f256f30b5a8, indexId=2662, indexOK=false,
    snapshot=0x0, nkeys=1, key=0x7ffe230f0180)
    at ../../Desktop/pg_src/src7/postgresql/src/backend/access/index/genam.c:413
(More stack frames follow...)

-------------------------
Hardware watchpoint 13: CatalogSnapshot

Old value = (Snapshot) 0x55ba7980b6a0 <CatalogSnapshotData>
New value = (Snapshot) 0x0
InvalidateCatalogSnapshot () at
../../Desktop/pg_src/src7/postgresql/src/backend/utils/time/snapmgr.c:435
435                     SnapshotResetXmin();
(gdb) bt 4
#0  InvalidateCatalogSnapshot ()
    at ../../Desktop/pg_src/src7/postgresql/src/backend/utils/time/snapmgr.c:435
#1  0x000055ba78f0ee85 in AtEOXact_Snapshot (isCommit=true, resetXmin=false)
    at ../../Desktop/pg_src/src7/postgresql/src/backend/utils/time/snapmgr.c:1057
#2  0x000055ba7868201b in CommitTransaction ()
    at ../../Desktop/pg_src/src7/postgresql/src/backend/access/transam/xact.c:2373
#3  0x000055ba78683495 in CommitTransactionCommand ()
    at ../../Desktop/pg_src/src7/postgresql/src/backend/access/transam/xact.c:3061
(More stack frames follow...)

--
but the whole process changes pg_class, pg_index,
pg_attribute,pg_constraint etc.
only one GetCatalogSnapshot and  InvalidateCatalogSnapshot seems not correct?
what if there are concurrency changes in the related pg_catalog table.

your patch did pass the isolation test!

I think you patch doing is against following code comments in
src/backend/utils/time/snapmgr.c

/*
 * CurrentSnapshot points to the only snapshot taken in transaction-snapshot
 * mode, and to the latest one taken in a read-committed transaction.
 * SecondarySnapshot is a snapshot that's always up-to-date as of the current
 * instant, even in transaction-snapshot mode.  It should only be used for
 * special-purpose code (say, RI checking.)  CatalogSnapshot points to an
 * MVCC snapshot intended to be used for catalog scans; we must invalidate it
 * whenever a system catalog change occurs.
 *
 * These SnapshotData structs are static to simplify memory allocation
 * (see the hack in GetSnapshotData to avoid repeated malloc/free).
 */
static SnapshotData CurrentSnapshotData = {SNAPSHOT_MVCC};
static SnapshotData SecondarySnapshotData = {SNAPSHOT_MVCC};
SnapshotData CatalogSnapshotData = {SNAPSHOT_MVCC};
SnapshotData SnapshotSelfData = {SNAPSHOT_SELF};
SnapshotData SnapshotAnyData = {SNAPSHOT_ANY};