Re: Add index scan progress to pg_stat_progress_vacuum

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: "Imseih (AWS), Sami" <simseih@amazon.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, Peter Geoghegan <pg@bowt.ie>, "Bossart, Nathan" <bossartn@amazon.com>, Justin Pryzby <pryzby@telsasoft.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2022-03-22T03:28:47Z
Lists: pgsql-hackers
Hi,

On 2022-03-16 21:47:49 +0000, Imseih (AWS), Sami wrote:
> From 85c47dfb3bb72f764b9052e74a7282c19ebd9898 Mon Sep 17 00:00:00 2001
> From: "Sami Imseih (AWS)" <simseih@amazon.com>
> Date: Wed, 16 Mar 2022 20:39:52 +0000
> Subject: [PATCH 1/1] Add infrastructure for parallel progress reporting
> 
> Infrastructure to allow a parallel worker to report
> progress. In a PARALLEL command, the workers and
> leader can report progress using a new pgstat_progress
> API.

What happens if we run out of memory for hashtable entries?


> +void
> +pgstat_progress_update_param_parallel(int leader_pid, int index, int64 val)
> +{
> +	ProgressParallelEntry *entry;
> +	bool found;
> +
> +	LWLockAcquire(ProgressParallelLock, LW_EXCLUSIVE);
> +
> +	entry = (ProgressParallelEntry *) hash_search(ProgressParallelHash, &leader_pid, HASH_ENTER, &found);
> +
> +	/*
> +	 * If the entry is not found, set the value for the index'th member,
> +	 * else increment the current value of the index'th member.
> +	 */
> +	if (!found)
> +		entry->st_progress_param[index] = val;
> +	else
> +		entry->st_progress_param[index] += val;
> +
> +	LWLockRelease(ProgressParallelLock);
> +}

I think that's an absolute no-go. Adding locking to progress reporting,
particularly a single central lwlock, is going to *vastly* increase the
overhead incurred by progress reporting.

Greetings,

Andres Freund



Commits

  1. Report index vacuum progress.

  2. Add new parallel message type to progress reporting.

  3. Remove MaxBackends variable in favor of GetMaxBackends() function.