Function modification visibility in parallel connection
<div><div>Hello guys.</div><div>I'm facing the following problem which seems to me a bug. Postgres version used - 13.7</div><div> </div><div>1) Connection1:</div><div>create or replace function test_fn() returns text as $$</div><div>begin</div><div> return 'version1';</div><div>end;</div><div>$$ language plpgsql;</div><div> </div><div>2) Connection2:</div><div>begin;</div><div>select test_fn();</div><div> test_fn</div><div>---------</div><div> version1</div><div>(1 row)</div><div> </div><div>3) Connection1:</div><div>create or replace function test_fn() returns text as $$</div><div>begin</div><div> return 'version2';</div><div>end;</div><div>$$ language plpgsql;</div><div> </div><div>4) Connection2:</div><div>select test_fn();</div><div> test_fn</div><div>---------</div><div> version1</div><div>(1 row)</div><div> </div><div>So the question is why Connection2 does not see updated version of test_fn? I've looked at source code and found that system cache invalidations (PROCOID cache in this case) occurs in AcceptInvalidationMessages() call chain. This function is called in a plenty of places but not when we call function in a running transaction as in the example. My thought is that AcceptInvalidationMessages() should be called in xact.c --> StartTransactionCommand() for TBLOCK_INPROGRESS, TBLOCK_IMPLICIT_INPROGRESS, TBLOCK_SUBINPROGRESS cases. Am I right or is it not even a bug?</div></div>
=?utf-8?B?0KHQtdC80ZHQvdC+0LIg0JzQuNGF0LDQuNC7?= <m7onov@yandex.ru> writes:
[ squishy behavior with function redefinitions ]
Yeah, if you just do "select foo(...);" over and over within an open
transaction, we won't notice any concurrent redefinitions of foo(),
because AcceptInvalidationMessages never gets called. To a first
approximation, there are two cases where we call
AcceptInvalidationMessages:
1. StartTransaction.
2. When opening (acquiring some lock on) a relation.
(I'm oversimplifying, but that will do for this discussion.) As long as
you don't submit a query that references a table, nor one that provokes
any new catalog fetch, no AcceptInvalidationMessages happens ... at least
not till some other session decides we're clogging the sinval queue
and pokes us with a catchup interrupt.
Given that it's been like this since roughly the late bronze age
with few complaints, there's a reasonable argument for doing nothing.
However, it's hard to deny that this isn't pretty inconsistent.
A "bulletproof" fix would involve adding the same amount of locking
overhead for functions as we have for tables; which has been discussed
but I think nobody wants to go there. Functions are basically defined
by just one catalog entry, so they don't have anywhere near the hazard
of seeing a partially-inconsistent definition as we'd have with tables
if we weren't paranoid about locks.
I wonder though if it could make sense to add AcceptInvalidationMessages
to what StartTransactionCommand() does in the INPROGRESS cases. That
would seem to bring things to parity with what happens for statements
that aren't inside a transaction block, and the overhead wouldn't be
very high (I think).
I notice that pgstat_init_function_usage recently grew some very
grotty hacking including its own AcceptInvalidationMessages call [1]https://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=5891c7a8ed8.
I wonder if it'd be acceptable to drop that if we added this.
It's not like that code is entirely race-condition-free anyway,
and we'd get away from the semantic inconsistency complained of
in its comments.
regards, tom lane
[1]: https://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=5891c7a8ed8
I wonder though if it could make sense to add AcceptInvalidationMessages
to what StartTransactionCommand() does in the INPROGRESS cases. That
would seem to bring things to parity with what happens for statements
that aren't inside a transaction block, and the overhead wouldn't be
very high (I think)
Thanks Tom!
Now I’m going to make a patch and suggest it to pgsql-hackers members as soon as I can.