Re: autovacuum and default_transaction_isolation

Dan Ports <drkp(at)csail(dot)mit(dot)edu> writes:
> I was doing some tests recently with default_transaction_isolation set
> to 'serializable' in postgresql.conf when I noticed pg_locks filling up
> with SIReadLocks rather more quickly than I expected.

> After some investigation, I found that an autovacuum worker was
> starting a transaction at the default isolation level. While using a
> serializable transaction doesn't affect its behavior (because it's not
> using a MVCC snapshot), having a serializable transaction open prevents
> other concurrent serializable transactions and their predicate locks
> from being cleaned up. Since VACUUM on a large table can take a long
> time, this could affect many concurrent transactions.

> My one-liner fix for this was to set
> DefaultXactIsoLevel = XACT_READ_COMMITTED;
> in AutoVacWorkerMain.

I've applied a patch for this in HEAD and 9.1, and also corrected what
seems to be a pre-existing bug that the autovac launcher did not force
zero_damaged_pages and statement_timeout off as autovac workers do.
I did not however touch the similar logic about synchronous_commit,
because I think that's got more problems than this.

Question 1: do we need to back-patch any of these changes further than
9.1?

Question 2: isn't this code broken?

if (synchronous_commit > SYNCHRONOUS_COMMIT_LOCAL_FLUSH)
SetConfigOption("synchronous_commit", "local",
PGC_SUSET, PGC_S_OVERRIDE);

The reason that Dan's one-liner fix isn't good enough, and we instead
need the applied two-liner fix

SetConfigOption("default_transaction_isolation", "read committed",
PGC_SUSET, PGC_S_OVERRIDE);

is that we have to make sure that guc.c knows about the change and will
honor it during subsequent activity. In particular, if SIGHUP
processing were to read a new value of default_transaction_isolation
from postgresql.conf, it would happily override a setting that had
simply been poked into the variable as Dan did it. We need to apply the
desired setting at PGC_S_OVERRIDE level to make sure that it can't be
changed later no matter what.

Because of this, I don't think I believe the above-quoted code for
adjusting synchronous_commit: it's not robust against the possibility of
the DBA changing the setting in postgresql.conf during the lifespan of
the autovac process. I could believe this:

if (synchronous_commit >= SYNCHRONOUS_COMMIT_LOCAL_FLUSH)
SetConfigOption("synchronous_commit", "local",
PGC_SUSET, PGC_S_OVERRIDE);
else
SetConfigOption("synchronous_commit", "off",
PGC_SUSET, PGC_S_OVERRIDE);

but I wonder why we are bothering, and not just forcing it to "off".
The above would still not do exactly what users might expect if the
DBA flips between "local" and "off", so what is it we're trying to
accomplish here? If it is worth worrying about, should the autovac
launcher be doing it too?

And, as mentioned, does anyone think any of these issues are significant
before 9.1?

regards, tom lane