On Mon, Jan 17, 2011 at 4:08 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Simon Riggs <si...@2ndquadrant.com> writes:
>> On Mon, 2011-01-17 at 14:46 -0500, Tom Lane wrote:
>>> Do we actually need a lock timeout either? The patch that was being
>>> discussed just involved failing if you couldn't get it immediately.
>>> I suspect that's sufficient for AV. At least, nobody's made a
>>> compelling argument why we need to expend a very substantially larger
>>> amount of work to do something different.
>
>> I have a patch to do this BUT the log message is harder. Until we grab
>> the lock we can't confirm the table still exists, so we can't get the
>> name for it. The whole point of this is logging the names of tables for
>> which we have failed to AV. Suggestions?
>
> As I said before, the correct place to fix this is in autovacuum.c,
> which has the table name near at hand. If you insist on fixing it
> somewhere else it's going to be very invasive.
Unless I'm missing something, making autovacuum.c call
ConditionalLockRelationOid() is not going to work, because the vacuum
transaction isn't started until we get all the way down to
vacuum_rel(). But it seems not too hard to make vacuum_rel() and
analyze_rel() have the proper relation name - we just need to
manufacture a suitable RangeVar to pass via the VacuumStmt's
"relation" element.
See what you think of the attached... I think logging this message at
LOG unconditionally is probably log spam of the first order, but if
the basic concept is sound we can argue over what to do about that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 7bc5f11..618051a 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -148,7 +148,18 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
* matter if we ever try to accumulate stats on dead tuples.) If the rel
* has been dropped since we last saw it, we don't need to process it.
*/
- onerel = try_relation_open(relid, ShareUpdateExclusiveLock);
+ if (!(vacstmt->options & VACOPT_NOWAIT))
+ onerel = try_relation_open(relid, ShareUpdateExclusiveLock);
+ else if (ConditionalLockRelationOid(relid, ShareUpdateExclusiveLock))
+ onerel = try_relation_open(relid, NoLock);
+ else
+ {
+ onerel = NULL;
+ ereport(LOG,
+ (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
+ errmsg("skipping analyze of \"%s\" --- lock not available",
+ vacstmt->relation->relname)));
+ }
if (!onerel)
return;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 5663711..7ac073b 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -61,7 +61,7 @@ static BufferAccessStrategy vac_strategy;
/* non-export function prototypes */
static List *get_rel_oids(Oid relid, const RangeVar *vacrel);
static void vac_truncate_clog(TransactionId frozenXID);
-static void vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast,
+static bool vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast,
bool for_wraparound, bool *scanned_all);
@@ -226,8 +226,11 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
bool scanned_all = false;
if (vacstmt->options & VACOPT_VACUUM)
- vacuum_rel(relid, vacstmt, do_toast, for_wraparound,
- &scanned_all);
+ {
+ if (!vacuum_rel(relid, vacstmt, do_toast, for_wraparound,
+ &scanned_all))
+ continue;
+ }
if (vacstmt->options & VACOPT_ANALYZE)
{
@@ -764,7 +767,7 @@ vac_truncate_clog(TransactionId frozenXID)
*
* At entry and exit, we are not inside a transaction.
*/
-static void
+static bool
vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
bool *scanned_all)
{
@@ -835,14 +838,28 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
*
* There's a race condition here: the rel may have gone away since the
* last time we saw it. If so, we don't need to vacuum it.
+ *
+ * If we've been asked not to wait for the relation lock, acquire it
+ * first in non-blocking mode, before calling try_relation_open().
*/
- onerel = try_relation_open(relid, lmode);
+ if (!(vacstmt->options & VACOPT_NOWAIT))
+ onerel = try_relation_open(relid, lmode);
+ else if (ConditionalLockRelationOid(relid, lmode))
+ onerel = try_relation_open(relid, NoLock);
+ else
+ {
+ onerel = NULL;
+ ereport(LOG,
+ (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
+ errmsg("skipping vacuum of \"%s\" --- lock not available",
+ vacstmt->relation->relname)));
+ }
if (!onerel)
{
PopActiveSnapshot();
CommitTransactionCommand();
- return;
+ return false;
}
/*
@@ -873,7 +890,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
relation_close(onerel, lmode);
PopActiveSnapshot();
CommitTransactionCommand();
- return;
+ return false;
}
/*
@@ -890,7 +907,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
relation_close(onerel, lmode);
PopActiveSnapshot();
CommitTransactionCommand();
- return;
+ return false;
}
/*
@@ -905,7 +922,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
relation_close(onerel, lmode);
PopActiveSnapshot();
CommitTransactionCommand();
- return;
+ return false;
}
/*
@@ -989,6 +1006,9 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
* Now release the session-level lock on the master table.
*/
UnlockRelationIdForSession(&onerelid, lmode);
+
+ /* Report that we really did it. */
+ return true;
}
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7bacf9b..b030b42 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2671,19 +2671,26 @@ autovacuum_do_vac_analyze(autovac_table *tab,
BufferAccessStrategy bstrategy)
{
VacuumStmt vacstmt;
+ RangeVar rangevar;
- /* Set up command parameters --- use a local variable instead of palloc */
+ /* Set up command parameters --- use local variables instead of palloc */
MemSet(&vacstmt, 0, sizeof(vacstmt));
+ MemSet(&rangevar, 0, sizeof(rangevar));
+
+ rangevar.schemaname = tab->at_nspname;
+ rangevar.relname = tab->at_relname;
+ rangevar.location = -1;
vacstmt.type = T_VacuumStmt;
- vacstmt.options = 0;
+ vacstmt.options = VACOPT_NOWAIT;
if (tab->at_dovacuum)
vacstmt.options |= VACOPT_VACUUM;
if (tab->at_doanalyze)
vacstmt.options |= VACOPT_ANALYZE;
vacstmt.freeze_min_age = tab->at_freeze_min_age;
vacstmt.freeze_table_age = tab->at_freeze_table_age;
- vacstmt.relation = NULL; /* not used since we pass a relid */
+ /* we pass the OID, but might need this anyway for an error message */
+ vacstmt.relation = &rangevar;
vacstmt.va_cols = NIL;
/* Let pgstat know what we're doing */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 483f225..d7d1b0a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2332,7 +2332,8 @@ typedef enum VacuumOption
VACOPT_ANALYZE = 1 << 1, /* do ANALYZE */
VACOPT_VERBOSE = 1 << 2, /* print progress info */
VACOPT_FREEZE = 1 << 3, /* FREEZE option */
- VACOPT_FULL = 1 << 4 /* FULL (non-concurrent) vacuum */
+ VACOPT_FULL = 1 << 4, /* FULL (non-concurrent) vacuum */
+ VACOPT_NOWAIT = 1 << 5
} VacuumOption;
typedef struct VacuumStmt
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
