On Fri, Jul 8, 2016 at 1:52 PM, Andres Freund <andres@anarazel.de> wrote:
> I'm a bit confused, why aren't we simply adding LSN interlock
> checks for toast? Doesn't look that hard? Seems like a much more
> natural course of fixing this issue?
I took some time trying to see what you have in mind, and I'm
really not "getting it". I definitely applaud you for spotting the
problem, but this suggestion for solving it doesn't seem to be
useful.
Basically, after turning this suggestion every way I could, I see
two alternative ways to implement it.
(1) Whenever TestForOldSnapshot() checks a heap page, check
whether the related toast is OK for all visible tuples on that
page. It would be enough to check one toast tuple for one value
per heap tuple, but still -- this would be really nasty from a
performance perspective.
(2) To deal with the fact that only about 7% of the
BufferGetPage() calls need to make this check, all functions and
macros which read toast data from the table would need extra
parameters, and all call sites for the toast API would need to have
such context information passed to them, so they could specify this
correctly. Ugh.
Compared to those options, the approach I was taking, where the fix
is "automatic" but some workloads where old_snapshot_threshold is
on would sequentially read some toast indexes more often seems
pretty tame.
Do you see some other option that fits what you describe? I'll
give you a couple days to respond before coding the patch.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company