Unexpected log chatter during transaction abort in CVS HEAD

Started by Tom Laneover 18 years ago1 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

About two weeks ago we had a discussion that concluded that notice
messages put out by GUC assign hooks should be logged when there's
a problem with a postgresql.conf setting, leading to this patch:
http://archives.postgresql.org/pgsql-committers/2007-12/msg00298.php

I noticed today that running pg_dump now causes a message like this
in the postmaster log:
LOG: SET TRANSACTION ISOLATION LEVEL must be called before any query
and investigation shows it's because of that patch. pg_dump allows
its serializable transaction to be rolled back at exit, and the
assign hook for XactIsoLevel has

if (SerializableSnapshot != NULL)
{
ereport(GUC_complaint_elevel(source),
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg("SET TRANSACTION ISOLATION LEVEL must be called before any query")));
/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
if (source != PGC_S_OVERRIDE)
return NULL;
}

and as you can probably tell from the comment, AtEOXact_GUC passes
source == PGC_S_OVERRIDE when it's trying to roll back a
within-transaction setting. So we'd better do something about that,
or we're going to see a lot of complaints about unexpected log messages.

One possibility is to change the logic to

if (SerializableSnapshot != NULL)
{
/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
if (source != PGC_S_OVERRIDE)
{
ereport(GUC_complaint_elevel(source),
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg("SET TRANSACTION ISOLATION LEVEL must be called before any query")));
return NULL;
}
}

but this seems a bit ugly, mainly because quite a few places would have
to be touched. I'm considering leaving the assign hooks as-is and
making GUC_complaint_elevel() return DEBUG5 for source ==
PGC_S_OVERRIDE, which would hide the chatter for all but the most
verbose logging.

Thoughts?

regards, tom lane