Обсуждение: cache invalidation skip logic

In cache invalidation logic, we have the following comment:

/*
* Now that we have the lock, check for invalidation messages, so that we
* will update or flush any stale relcache entry before we try to use it.
* RangeVarGetRelid() specifically relies on us for this.  We can skip
* this in the not-uncommon case that we already had the same type of lock
* being requested, since then no one else could have modified the
* relcache entry in an undesirable way.  (In the case where our own xact
* modifies the rel, the relcache update happens via
* CommandCounterIncrement, not here.)
*/
if (res != LOCKACQUIRE_ALREADY_HELD)  AcceptInvalidationMessages();

It is true after we hold the lock, nobody will further modify it but there
could be some left-over invalidation message we shall accept before we can
continue. This is can be demonstrated with the following invalidation
sequence:{       1: inval A;       2: inval B;       ...;       10: inval pg_class
}

After step 10, another session may encounter a lock and replays this sequence:

step 1:  RelationBuildDesc(A), it heap_open(pg_class),        pg_class lock not acquired yet, so it acquires the lock
and       recursively replay the sequence, goto step 2.
 
step 2:        RelationBuildDesc(B), it heap_open(pg_class),        but this time we already have
LOCKACQUIRE_ALREADY_HELDwith        pg_class, so we now access pg_class but it is wrong.
 

User may ends up with a "could not open file ..." error.

Is above sequence possible?

Regards,
Qingqing

On Thu, Aug 6, 2015 at 2:19 PM, Qingqing Zhou <zhouqq.postgres@gmail.com> wrote:
> In cache invalidation logic, we have the following comment:
>
> /*
> * Now that we have the lock, check for invalidation messages, so that we
> * will update or flush any stale relcache entry before we try to use it.
> * RangeVarGetRelid() specifically relies on us for this.  We can skip
> * this in the not-uncommon case that we already had the same type of lock
> * being requested, since then no one else could have modified the
> * relcache entry in an undesirable way.  (In the case where our own xact
> * modifies the rel, the relcache update happens via
> * CommandCounterIncrement, not here.)
> */
> if (res != LOCKACQUIRE_ALREADY_HELD)
>    AcceptInvalidationMessages();
>
> It is true after we hold the lock, nobody will further modify it but there
> could be some left-over invalidation message we shall accept before we can
> continue. This is can be demonstrated with the following invalidation
> sequence:
>  {
>         1: inval A;
>         2: inval B;
>         ...;
>         10: inval pg_class
> }
>
> After step 10, another session may encounter a lock and replays this sequence:
>
> step 1:  RelationBuildDesc(A), it heap_open(pg_class),
>          pg_class lock not acquired yet, so it acquires the lock and
>          recursively replay the sequence, goto step 2.
> step 2:
>          RelationBuildDesc(B), it heap_open(pg_class),
>          but this time we already have LOCKACQUIRE_ALREADY_HELD with
>          pg_class, so we now access pg_class but it is wrong.
>
> User may ends up with a "could not open file ..." error.
>
> Is above sequence possible?

In step 1, AcceptInvalidationMessages() should process all pending
invalidation messages.  So if step 2 did AcceptInvalidationMessages()
again it would be a no-op, because no messages should remain at that
point.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Sun, Aug 9, 2015 at 8:24 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> In step 1, AcceptInvalidationMessages() should process all pending
> invalidation messages.  So if step 2 did AcceptInvalidationMessages()
> again it would be a no-op, because no messages should remain at that
> point.
>

That's what I think at first. I would try to see if I can manually repro a case.

Thanks,
Qingqing