From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Dubious shortcut in ckpt_buforder_comparator()
Date: Wed, 10 Jan 2018 13:06:50 -0500
Message-ID: <18437.1515607610@sss.pgh.pa.us> (raw)
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: Dubious shortcut in ckpt_buforder_comparator()
In-Reply-To: <18437.1515607610@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox