From: | "Imseih (AWS), Sami" <simseih(at)amazon(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Add index scan progress to pg_stat_progress_vacuum |
Date: | 2023-04-10 19:20:42 |
Message-ID: | 1F9143C4-D709-4F37-8FF0-D9D5E0C38BB7@amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> + case 'P': /* Parallel progress reporting */
I kept this comment as-is but inside case code block I added
more comments. This is to avoid cluttering up the one-liner comment.
> + * Increase and report the number of index scans. Also, we reset the progress
> + * counters.
> The counters reset are the two index counts, perhaps this comment
> should mention this fact.
Yes, since we are using the multi_param API here, it makes sense to
mention the progress fields being reset in the comments.
+ /* update progress */
+ int index = pq_getmsgint(msg, 4);
+ int incr = pq_getmsgint(msg, 1);
[...]
+ pq_beginmessage(&progress_message, 'P');
+ pq_sendint32(&progress_message, index);
+ pq_sendint64(&progress_message, incr);
+ pq_endmessage(&progress_message);
> It seems to me that the receiver side is missing one pq_getmsgend()?
Yes. I added this.
> incr is defined and sent as an int64 on the sender side, hence the
> receiver should use pq_getmsgint64(), no? pq_getmsgint(msg, 1) means
> to receive only one byte, see pqformat.c.
Ah correct, incr is an int64 so what we need is.
int64 incr = pq_getmsgint64(msg);
I also added the pq_getmsgend call.
> And the order is reversed?
I don't think so. The index then incr are sent and they are
back in the same order. Testing the patch shows the value
increments correctly.
See v28 addressing the comments.
Regards,
Sami Imseih
AWS (Amazon Web Services)
Attachment | Content-Type | Size |
---|---|---|
v28-0001-Report-index-vacuum-progress.patch | application/octet-stream | 13.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2023-04-10 20:39:28 | Re: When to drop src/tools/msvc support |
Previous Message | Peter Geoghegan | 2023-04-10 19:18:27 | Re: Show various offset arrays for heap WAL records |