Re: Add index scan progress to pg_stat_progress_vacuum - Mailing list pgsql-hackers

From Imseih (AWS), Sami
Subject Re: Add index scan progress to pg_stat_progress_vacuum
Date
Msg-id 1F9143C4-D709-4F37-8FF0-D9D5E0C38BB7@amazon.com
Whole thread Raw
In response to Re: Add index scan progress to pg_stat_progress_vacuum  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Add index scan progress to pg_stat_progress_vacuum
List 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

pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: Show various offset arrays for heap WAL records
Next
From: Magnus Hagander
Date:
Subject: Re: When to drop src/tools/msvc support