Re: [HACKERS] Commits 8de72b and 5457a1 (COPY FREEZE)

Noah Misch <noah(at)leadboat(dot)com> writes:
> On Wed, May 01, 2019 at 07:01:23PM -0400, Tom Lane wrote:
>> The thing I was worried about in RelationCacheInvalidate does seem
>> to be a red herring, at least fixing it is not necessary to make
>> the broken-session-state problem go away.

> Your earlier proposal would have made RelationCacheInvalidate() work more like
> RelationFlushRelation() when rd_newRelfilenodeSubid is set. That's a good
> direction, all else being equal, though I'm not aware of a specific bug
> reachable today. I think RelationCacheInvalidate() would then need the
> reference count bits that RelationFlushRelation() has.

Yeah. I'm not actually convinced that treating rd_createSubid and
rd_newRelfilenodeSubid alike here is appropriate, though. If
rd_createSubid is set then we certainly can assume that no other sessions
can see/modify the relation, but we cannot make the same assumption when
rd_newRelfilenodeSubid is set. The comment argues, in essence, that it's
okay if we have AEL on the relation, but I'm not 100% convinced about
that ... still, I can't construct a counterexample at the moment.

> Why this order change?

Because of the comment just above:

* ... Maintain the order that they
* would be processed in by AtEOXact_Inval(), to ensure emulated behaviour
* in redo is as similar as possible to original. We want the same bugs,
* if any, not new ones.

In principle the order of processing inval events should not matter (if it
does, then this patch is much more dangerous than it looks). But I concur
with this comment that it's best if standby servers apply the events in
the same order the master would; and this patch does cause that order to
change.

regards, tom lane