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};