Re: explain format json, unit for serialize and memory are different.

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, jian he <jian(dot)universality(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: explain format json, unit for serialize and memory are different.
Date: 2024-05-14 10:33:15
Message-ID: CAApHDvrqhi6gkO9bdvASchKQazwYf6o2Csy=SnrzMtt6Acpgdg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 14 May 2024 at 18:16, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> I think for v17, we should consider adding a macro to explain.c to
> calculate the KB from bytes. There are other inconsistencies that it
> would be good to address. We normally round up to the nearest kilobyte
> with (bytes + 1023) / 1024, but if you look at what 06286709e did, it
> seems to be rounding to the nearest without any apparent justification
> as to why. It does (metrics->bytesSent + 512) / 1024.
>
> show_memory_counters() could be modified to use the macro and show
> kilobytes rather than bytes.

Here's a patch for that.

I checked the EXPLAIN SERIALIZE thread and didn't see any mention of
the + 512 thing. It seems Tom added it just before committing and no
patch ever made it to the mailing list with + 512. The final patch on
the list is in [1].

For the EXPLAIN MEMORY part, the bytes vs kB wasn't discussed. The
closest the thread came to that was what Abhijit mentioned in [2].

I also adjusted some inconsistencies around spaces between the digits
and kB. In other places in EXPLAIN we write "100kB" not "100 kB". I
see we print times with a space ("Execution Time: 1.719 ms"), so we're
not very consistent overall, but since the EXPLAIN MEMORY is new, it
makes sense to change it now to be aligned to the other kB stuff in
explain.c

The patch does change a long into an int64 in show_hash_info(). I
wondered if that part should be backpatched. It does not seem very
robust to me to divide a Size by 1024 and expect it to fit into a
long. With MSVC 64 bit, sizeof(Size) == 8 and sizeof(long) == 4. I
understand work_mem is limited to 2GB on that platform, but it does
not seem like a good reason to use a long.

David

[1] https://www.postgresql.org/message-id/CAEze2WhopAFRS4xdNtma6XtYCRqydPWAg83jx8HZTowpeXzOyg%40mail.gmail.com
[2] https://www.postgresql.org/message-id/ZaF1fB_hMqycSq-S%40toroid.org

Attachment Content-Type Size
v1-0001-Minor-fixes-to-EXPLAIN.patch application/octet-stream 6.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Elena Indrupskaya 2024-05-14 10:34:56 Re: First draft of PG 17 release notes
Previous Message Ilyasov Ian 2024-05-14 10:22:29 Fix src/test/subscription/t/029_on_error.pl test when wal_debug is enabled