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
