Horton

Thread

Commits

  1. Remove dubious micro-optimization in ckpt_buforder_comparator().

  1. Dubious shortcut in ckpt_buforder_comparator()

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-01-10T18:06:50Z

    While analyzing a recent crash report[1], I noticed that bufmgr.c's
ckpt_buforder_comparator is coded to assume that no two CkptSortItems
could have equal page IDs; it therefore skips the final comparison
and will never return 0 (equal).  I do not think that assumption is
correct.  I do not see anything preventing the scenario where, while
we are scanning the buffer pool at the beginning of BufferSync(),
somebody writes out a dirty buffer we've already seen and recycles
it for another page, and then somebody else fetches that same page
back into a different buffer and dirties it, and finally we arrive
at that second buffer and conclude it should be written too.

If we do get two entries with equal page IDs into the CkptSortItem
list, ckpt_buforder_comparator will give self-inconsistent results:
when actually x = y, it will say both x > y and y > x depending on
which way you pass the arguments to it.

Now, I cannot find any reason to think this would have awful consequences
given our current implementation of qsort.  But we might not always use
that code --- I remember some discussion of timsort, for instance ---
and another sorting implementation might be less happy about it.

So I think we should get rid of that micro-optimization, which is
probably useless anyway from a performance standpoint, and do the
comparison honestly.  Any objections?

			regards, tom lane

[1] https://postgr.es/m/CAMd+QORjQGFPhRqXj8DSLn2_UgnTHtP=Kc7h2e5_hGz0GLaBMw@mail.gmail.com

  2. Re: Dubious shortcut in ckpt_buforder_comparator()

    Andres Freund <andres@anarazel.de> — 2018-01-10T19:34:07Z

    Hi,

On 2018-01-10 13:06:50 -0500, Tom Lane wrote:
> So I think we should get rid of that micro-optimization, which is
> probably useless anyway from a performance standpoint, and do the
> comparison honestly.  Any objections?

No, absolutely none. You're going to change it?

Greetings,

Andres Freund

  3. Re: Dubious shortcut in ckpt_buforder_comparator()

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-01-10T19:57:51Z

    Andres Freund <andres@anarazel.de> writes:
> On 2018-01-10 13:06:50 -0500, Tom Lane wrote:
>> So I think we should get rid of that micro-optimization, which is
>> probably useless anyway from a performance standpoint, and do the
>> comparison honestly.  Any objections?

> No, absolutely none. You're going to change it?

Will do.

			regards, tom lane