Re: [HACKERS] couldn't rollback cache ? - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [HACKERS] couldn't rollback cache ?
Date
Msg-id 13769.937837688@sss.pgh.pa.us
Whole thread Raw
In response to RE: [HACKERS] couldn't rollback cache ?  ("Hiroshi Inoue" <Inoue@tpf.co.jp>)
Responses RE: [HACKERS] couldn't rollback cache ?  ("Hiroshi Inoue" <Inoue@tpf.co.jp>)
List pgsql-hackers
"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
> I think it's not done correctly for tuple SI messages either.
> I didn't use current cache invalidation mechanism when I made the
> patch for SearchSysCache() because of the following 2 reasons. 

> 1. SI messages are eaten by CommandCounterIncrement(). So they
>     may vanish before transaction end/aborts.

I think this is OK.  The sending backend does not send the SI message
in the first place until it has committed.  Other backends can read
the messages at CommandCounterIncrement; it doesn't matter whether the
other backends later commit or abort their own transactions.  I think.
Do you have a counterexample?

> 2. The tuples which should be invalidated in case of abort are different
>     from ones in case of commit.
>     In case of commit,deleting old tuples should be invalidated for all
>     backends.
>     In case of abort,insert(updat)ing new tuples should be invalidated 
>     for the insert(updat)ing backend.

I wonder whether it wouldn't be cleaner to identify the target tuples
by OID instead of ItemPointer.  That way would work for both new and
update tuples...

>     Currently heap_insert() calls RelationInvalidateHeapTuple() for a 
>     inserting new tuple but heap_replace() doesn't call RelationInvalid-
>     ateHeapTuple() for a updating new tuple. I don't understand which
>     is right.

Hmm.  Invalidating the old tuple is the right thing for heap_replace in
terms of sending a message to other backends at commit; it's the old
tuple that they might have cached and need to get rid of.  But for
getting rid of this backend's uncommitted new tuple in case of abort,
it's not the right thing.  OTOH, your change to add a time qual check
to SearchSysCache would fix that, wouldn't it?  But invalidating by OID
would make the issue moot.

Possibly heap_insert doesn't need to be calling
RelationInvalidateHeapTuple at all --- a new tuple can't be cached by
any other backend, by definition, until it has committed; so there's no
need to send out an SI message for it.  That call must be there to
ensure that the local cache gets purged of the tuple in case of abort.
Maybe we could remove that call (and reduce SI traffic) if we rely on
a time qual to purge bogus entries from the local caches after abort.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Vadim Mikheev
Date:
Subject: Re: [HACKERS] why do shmem attach?
Next
From: Tom Lane
Date:
Subject: Re: [HACKERS] why do shmem attach?