Use pgstat_progress_update_multi_p aram instead of single param update

Hi,

While reviewing progress reporting for COPY command patches [1], a
point on using pgstat_progress_update_multi_param instead of
pgstat_progress_update_param wherever possible was suggested in [1].
We could do multiple param updates at once with a single API than
doing each param update separately. The advantages are - 1) reducing
few function calls making the code look cleaner 2) atomically updating
multiple parameters at once within a single backend write critical
section  i.e. incrementing st_changecount at once in a critical
section instead of doing it for each param separately.

Attached is a patch that replaces some subsequent multiple
update_param calls with a single update_multi_param.

Thoughts?

[1] -

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

On Sun, Feb 21, 2021 at 11:30:21AM +0530, Bharath Rupireddy wrote:
> Attached is a patch that replaces some subsequent multiple
> update_param calls with a single update_multi_param.

Looks mostly fine to me.

-    if (OidIsValid(indexOid))
-        pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
-                                     PROGRESS_CLUSTER_COMMAND_CLUSTER);
-    else
-        pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
-                                     PROGRESS_CLUSTER_COMMAND_VACUUM_FULL);
+    pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
+                                 OidIsValid(indexOid) ? PROGRESS_CLUSTER_COMMAND_CLUSTER :
+                                 PROGRESS_CLUSTER_COMMAND_VACUUM_FULL);
What's the point of changing this one?
--
Michael

On Sun, Feb 21, 2021 at 4:18 PM Michael Paquier < > wrote:
While we are at it, I wanted to use a single line statement instead of
if else, just like we do it in do_analyze_rel as below.

    pgstat_progress_update_param(PROGRESS_ANALYZE_PHASE,
                                 inh ?
PROGRESS_ANALYZE_PHASE_ACQUIRE_SAMPLE_ROWS_INH :
                                 PROGRESS_ANALYZE_PHASE_ACQUIRE_SAMPLE_ROWS);

We can ignore it if it doesn't seem a good way.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

On Sun, Feb 21, 2021 at 04:43:23PM +0530, Bharath Rupireddy wrote:
> While we are at it, I wanted to use a single line statement instead of
> if else, just like we do it in do_analyze_rel as below.
>
>     pgstat_progress_update_param(PROGRESS_ANALYZE_PHASE,
>                                  inh ?
> PROGRESS_ANALYZE_PHASE_ACQUIRE_SAMPLE_ROWS_INH :
>                                  PROGRESS_ANALYZE_PHASE_ACQUIRE_SAMPLE_ROWS);
>
> We can ignore it if it doesn't seem a good way.

What's always annoying with such things is that they create conflicts
with back-branches.  So I have removed it, and applied the rest.
Thanks!
--
Michael