Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups

Robert Haas wrote:
> On Thu, Jan 19, 2017 at 10:06 AM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)com> wrote:
> > Peter Eisentraut wrote:
> >
> >> Also, I wonder whether we should not in vacuum.c change the order of the
> >> calls of SetTransactionIdLimit() and SetMultiXactIdLimit() as well, just
> >> to keep everything consistent.
> >
> > I am wary of doing that. The current coding is well battle-tested by
> > now, but doing things in the opposite order, not at all. Pending some
> > testing to show that there is no problem with a change, I would leave
> > things as they are. Probably said testing is too onerous for the
> > benefit (which is just a little consistency). What I fear is: what
> > happens if a concurrent checkpoint reads the values between the two
> > operations, and a crash occurs? I think that the checkpoint might save
> > the updated values, so after crash recovery the truncate would not be
> > executed, possibly leaving files around. Leaving files around might be
> > dangerous for multixacts at least (it's probably harmless for xids).
>
> Don't both SLRUs eventually wrap? If so, leaving file around seems
> dangerous for either in equal measure.

Well, multixact uses the whole 2^32 space as valid, while xid only uses
half of it. I think you would have to crash on every checkpoint just
between those two points for two billion xacts for there to be a problem
for xids. Anyway this just reinforces my argument that we shouldn't
move those calls -- moving only the commit_ts one is good.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services