agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Justin Pryzby <pryzby@telsasoft.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Why does [auto-]vacuum delay not report a wait event?
Date: Sun, 22 Mar 2020 09:38:29 -0500
Message-ID: <20200322143829.GD2563@telsasoft.com> (raw)
In-Reply-To: <20200322002457.dvepqpj4legcmuxn@alap3.anarazel.de>
References: <20200319224449.pbjrivflbf4x76tj@alap3.anarazel.de>
<20200321040750.GD13662@telsasoft.com>
<20200322002457.dvepqpj4legcmuxn@alap3.anarazel.de>
On Sat, Mar 21, 2020 at 05:24:57PM -0700, Andres Freund wrote:
> > Also, I noticed that SLEEP_ON_ASSERT comment (31338352b) wants to use pg_usleep
> > "which seems too short.". Surely it should use pg_sleep, added at 782eefc58 a
> > few years later ?
>
> I don't see problem with using sleep here?
There's no problem with pg_sleep (with no "u") - it just didn't exist when
SLEEP_ON_ASSERT was added (and I guess it's potentially unsafe to do much of
anything, like loop around pg_usleep(1e6)). I'm suggesting it *should* use
pg_sleep, rather than explaining why pg_usleep (with a "u") doesn't work.
> > Also, there was a suggestion recently that this should have a separate
> > vacuum_progress phase:
> > |vacuumlazy.c:#define VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL 50 /* ms */
> > |vacuumlazy.c:pg_usleep(VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL * 1000L);
> >
> > I was planning to look at that eventually ; should it have a wait event instead
> > or in addition ?
>
> A separate phase? How would that look like? We don't want to replace the
> knowledge that currently e.g. the heap scan is in progress?
I don't think that's an issue, since the heap scan is done at that point ?
heap_vacuum_rel() (the publicly callable routine) calls lazy_scan_heap (which
does everything) and then (optionally) lazy_truncate_heap() and then
immediately afterwards does:
pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_PHASE_FINAL_CLEANUP);
...
pgstat_progress_end_command();
> > VACUUM VERBOSE wouldn't normally be run with cost_delay > 0, so that field will
> > typically be zero, so I made it conditional
>
> I personally dislike conditional output like that, because it makes
> parsing the output harder.
I dislike it too, mostly because there's a comment explaining why it's done
like that, without any desirable use of the functionality. If it's not useful
for a case where the field is typically zero, it should go away until its
utility is instantiated.
--
Justin
view thread (11+ messages) latest in thread
Message-ID: <20200322143829.GD2563@telsasoft.com>
Permalink: ../20200322143829.GD2563@telsasoft.com/
Also on: postgresql.org/message-id/20200322143829.GD2563@telsasoft.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, andres@anarazel.de
Subject: Re: Why does [auto-]vacuum delay not report a wait event?
In-Reply-To: <20200322143829.GD2563@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox