pgsql: Split out tiebreaker comparisons from comparetup_* functions

Started by John Nayloralmost 3 years ago6 messagescomitters
Jump to latest
#1John Naylor
john.naylor@enterprisedb.com

Split out tiebreaker comparisons from comparetup_* functions

Previously, if a specialized comparator found equal datum1 keys,
the "comparetup" function would repeat the comparison on the
datum before proceeding with the unabbreviated first key
and/or additional sort keys.

Move comparing additional sort keys into "tiebreak" functions so
that specialized comparators can call these directly if needed,
avoiding duplicate work.

Reviewed by David Rowley

Discussion: /messages/by-id/CAFBsxsGaVfUrjTghpf=kDBYY=jWx1PN-fuusVe7Vw5s0XqGdGw@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/c9bfa40914be4a0882b904834c2bd32775a50d7b

Modified Files
--------------
src/backend/utils/sort/tuplesort.c | 9 +--
src/backend/utils/sort/tuplesortvariants.c | 125 ++++++++++++++++++++++-------
src/include/utils/tuplesort.h | 7 ++
3 files changed, 108 insertions(+), 33 deletions(-)

#2John Naylor
john.naylor@enterprisedb.com
In reply to: John Naylor (#1)
Re: pgsql: Split out tiebreaker comparisons from comparetup_* functions

On Wed, Aug 16, 2023 at 5:17 PM John Naylor <john.naylor@postgresql.org>
wrote:

Split out tiebreaker comparisons from comparetup_* functions

Hammerkop is getting OOM errors with this change. Most grand totals in the
memory context stats show between 1 and 3 MB, and "tuplesort main" only
shows small-ish requests, so not sure what's going on.

--
John Naylor
EDB: http://www.enterprisedb.com

#3John Naylor
john.naylor@enterprisedb.com
In reply to: John Naylor (#2)
Re: pgsql: Split out tiebreaker comparisons from comparetup_* functions

On Thu, Aug 17, 2023 at 11:41 AM John Naylor <john.naylor@enterprisedb.com>
wrote:

On Wed, Aug 16, 2023 at 5:17 PM John Naylor <john.naylor@postgresql.org>

wrote:

Split out tiebreaker comparisons from comparetup_* functions

Hammerkop is getting OOM errors with this change. Most grand totals in

the memory context stats show between 1 and 3 MB, and "tuplesort main" only
shows small-ish requests, so not sure what's going on.

That remains the only new failure in the buildfarm, and it just failed in
the same way again. I'll try to figure out what could be the cause, but if
I don't I'll revert within 24 hours.

--
John Naylor
EDB: http://www.enterprisedb.com

#4Thomas Munro
thomas.munro@gmail.com
In reply to: John Naylor (#3)
Re: pgsql: Split out tiebreaker comparisons from comparetup_* functions

On Fri, Aug 18, 2023 at 12:48 AM John Naylor
<john.naylor@enterprisedb.com> wrote:

On Thu, Aug 17, 2023 at 11:41 AM John Naylor <john.naylor@enterprisedb.com> wrote:

On Wed, Aug 16, 2023 at 5:17 PM John Naylor <john.naylor@postgresql.org> wrote:

Split out tiebreaker comparisons from comparetup_* functions

Hammerkop is getting OOM errors with this change. Most grand totals in the memory context stats show between 1 and 3 MB, and "tuplesort main" only shows small-ish requests, so not sure what's going on.

That remains the only new failure in the buildfarm, and it just failed in the same way again. I'll try to figure out what could be the cause, but if I don't I'll revert within 24 hours.

I don't know what's going on on that machine but those two failures
seem different, and there are also now some failures in release
branches that don't have your commit, so I'd wait long for more
evidence...

#5John Naylor
john.naylor@enterprisedb.com
In reply to: Thomas Munro (#4)
Re: pgsql: Split out tiebreaker comparisons from comparetup_* functions

On Fri, Aug 18, 2023 at 5:26 PM Thomas Munro <thomas.munro@gmail.com> wrote:

Hammerkop is getting OOM errors with this change. Most grand totals

in the memory context stats show between 1 and 3 MB, and "tuplesort main"
only shows small-ish requests, so not sure what's going on.

That remains the only new failure in the buildfarm, and it just failed

in the same way again. I'll try to figure out what could be the cause, but
if I don't I'll revert within 24 hours.

I don't know what's going on on that machine but those two failures
seem different, and there are also now some failures in release
branches that don't have your commit, so I'd wait long for more
evidence...

Ah, I did check at least one back branch yesterday and didn't see any
failures then. I see them now, thanks!

(FWIW, I've been looking for signs of memory malfeasance in the commit, and
found nothing yet)

--
John Naylor
EDB: http://www.enterprisedb.com

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: John Naylor (#5)
Re: pgsql: Split out tiebreaker comparisons from comparetup_* functions

John Naylor <john.naylor@enterprisedb.com> writes:

On Fri, Aug 18, 2023 at 5:26 PM Thomas Munro <thomas.munro@gmail.com> wrote:

I don't know what's going on on that machine but those two failures
seem different, and there are also now some failures in release
branches that don't have your commit, so I'd wait long for more
evidence...

Ah, I did check at least one back branch yesterday and didn't see any
failures then. I see them now, thanks!

... and hamerkop just went back to green on HEAD. So whatever is up
with that machine, it's not the fault of your commit.

regards, tom lane