Jeff Janes <jeff.janes@gmail.com> wrote:
> I guess I'm a couple releases late to review the "autovacuum
> truncate exclusive lock" patch (a79ae0bc0d454b9f2c95a), but this
> patch did not only affect autovac, it affects manual vacuum as
> well (as did the original behavior it is a modification of). So
> the compiler constants are misnamed, and the elog message when it
> triggers is misleading. (Is it also misleading to just say
> "vacuum"? Does it need to say "(auto)vacuum"?)
I think it should because I don't think this heuristic is
appropriate for an explicit VACUUM command. It's one thing for
autovacuum to leave the file sizes alone if it can't truncate
without blocking foreground processes; it's another for a
foreground VACUUM request to not do all of the work it was expected
to do.
> I also log the number of pages truncated at the time it gave up,
> as it would be nice to know if it is completely starving or
> making some progress.
If we're going to have the message, we should make it useful. My
biggest question here is not whether we should add this info, but
whether it should be DEBUG instead of LOG.
> Also, I think that permanently boycotting doing autoanalyze
> because someone is camping out on an access share lock (or
> because there are a never-ending stream of overlapping locks) and
> so the truncation cannot be done is a bit drastic, especially for
> inclusion in a point release.
That much is not a change in the event that the truncation does not
complete. I have seen cases where the old logic head-banged for
hours or days without succeeding at the truncation attempt in
autovacuum, absolutely killing performance until the user ran an
explicit VACUUM. And in the meantime, since the deadlock detection
logic was killing autovacuum before it got to the analyze phase,
the autoanalyze was never done.
Perhaps the new logic should go ahead and get its lock even on a
busy system (like the old logic), and bail out after some
incremental progress. Based on the current reports, it seems
likely that at least some workloads are not allowing the current
code to get a toe-hold.
> Is there a way to have the autoanalyze happen, but then still
> arrange for the autovacuum to be triggered again next naptime?
I suggested that myself, but that was a bigger change from the old
behavior (at least if we allow that initial lock so we guarantee
some progress, per the above), and I didn't think that should be
back-patched. A separate patch for just that, applied to
master/HEAD, seems like a good idea to me.
--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
