On Fri, May 27, 2022 at 08:55:02AM -0700, Andres Freund wrote: > On 2022-05-27 15:44:39 +0530, Amit Kapila wrote: >> Won't in theory the similar cache in transam.c is also prone to >> similar behavior?
TransactionIdDidCommit() and TransactionIdDidAbort() are used in much more code paths for visibility purposes, contrary to the subtrans.c ones. > It's not quite the same risk, because there we are likely to actually hit the > cache regularly. Whereas quite normal workloads might not hit this cache for > days on end. Yeah. In short, this mostly depends on the use of savepoints and the number of XIDs issued until PGPROC_MAX_CACHED_SUBXIDS is reached, and a single cache entry in this code path would reduce the pressure on the SLRU lookups depending on the number of queries issued, for example. One thing I know of that likes to abuse of savepoints and could cause overflows to make this easier to hit is the ODBC driver coupled with short queries in long transactions, where its internals enforce the use of a savepoint each time a query is issued by an application (pretty much what the benchmark at the top of the thread does). In this case, even the single cache approach would not help much because I recall that we finish with one savepoint per query to be able to rollback to any previous state within a given transaction (as the ODBC APIs allow). Doing a caching within SubTransGetParent() would be more interesting, for sure, though the invalidation to clean the cache and to make that robust enough may prove tricky. It took me some time to come back to this thread. The change has now been reverted. -- Michael
signature.asc
Description: PGP signature
