Re: CreateFakeRelcacheEntry versus Hot Standby

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> I started looking at the callers of CreateFakeRelcacheEntry to see if
> any of them actually take a lock on the Relation. It seems not, so maybe
> we should just set those to InvalidOid, and add an
> Assert(OidIsValid(dbId) && OidIsValid(relId)) to LockAcquire() to ensure
> that a fake relcache entry is not used to grab locks in the future either.

+1, I was thinking that would be a reasonable thing to try. (You can't
assert dbID != 0, but the relid test should be sufficient.)

> However, I spotted another bug. In ginContinueSplit(), we call
> CreateFakeRelcacheEntry(), and pass the fake relcache entry to
> prepareEntryScan() or prepareDataScan(). Those functions store the
> passed-in Relation ptr in the passed in GinBtreeData struct. After the
> call, we free the fake relcache entry, but the pointer is still present
> in the GinBtreeData. The findParents() call later in the function uses
> it, and will thus access free'd memory.

> A trivial fix is to delay freeing the fake relcache entry in
> ginContinueSplit(). But this once again shows that our WAL redo
> functions are not as well tested as they should be, and the WAL redo
> cleanup functions in particularly.

Well, HS will at least help us shake out the redo functions. Cleanup
is a hard case to test ...

regards, tom lane