From: Tom Lane Date: Thu, 30 Oct 2014 17:03:34 +0000 (-0400) Subject: Test IsInTransactionChain, not IsTransactionBlock, in vac_update_relstats. X-Git-Tag: REL9_1_15~80 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fcf0246b2c58d4b7e480ccb11e1bdaeef023a4f6;p=postgresql Test IsInTransactionChain, not IsTransactionBlock, in vac_update_relstats. As noted by Noah Misch, my initial cut at fixing bug #11638 didn't cover all cases where ANALYZE might be invoked in an unsafe context. We need to test the result of IsInTransactionChain not IsTransactionBlock; which is notationally a pain because IsInTransactionChain requires an isTopLevel flag, which would have to be passed down through several levels of callers. I chose to pass in_outer_xact (ie, the result of IsInTransactionChain) rather than isTopLevel per se, as that seemed marginally more apropos for the intermediate functions to know about. --- diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index b5d1c617dd..0e876904d9 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -84,7 +84,8 @@ static MemoryContext anl_context = NULL; static BufferAccessStrategy vac_strategy; -static void do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, bool inh); +static void do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, + bool inh, bool in_outer_xact); static void BlockSampler_Init(BlockSampler bs, BlockNumber nblocks, int samplesize); static bool BlockSampler_HasMore(BlockSampler bs); @@ -116,7 +117,8 @@ static bool std_typanalyze(VacAttrStats *stats); * analyze_rel() -- analyze one relation */ void -analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy) +analyze_rel(Oid relid, VacuumStmt *vacstmt, + bool in_outer_xact, BufferAccessStrategy bstrategy) { Relation onerel; @@ -228,13 +230,13 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy) /* * Do the normal non-recursive ANALYZE. */ - do_analyze_rel(onerel, vacstmt, false); + do_analyze_rel(onerel, vacstmt, false, in_outer_xact); /* * If there are child tables, do recursive ANALYZE. */ if (onerel->rd_rel->relhassubclass) - do_analyze_rel(onerel, vacstmt, true); + do_analyze_rel(onerel, vacstmt, true, in_outer_xact); /* * Close source relation now, but keep lock so that no one deletes it @@ -257,7 +259,8 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy) * do_analyze_rel() -- analyze one relation, recursively or not */ static void -do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, bool inh) +do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, + bool inh, bool in_outer_xact) { int attr_cnt, tcnt, @@ -534,7 +537,10 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, bool inh) if (!inh) vac_update_relstats(onerel, RelationGetNumberOfBlocks(onerel), - totalrows, hasindex, InvalidTransactionId); + totalrows, + hasindex, + InvalidTransactionId, + in_outer_xact); /* * Same for indexes. Vacuum always scans all indexes, so if we're part of @@ -551,7 +557,10 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, bool inh) totalindexrows = ceil(thisdata->tupleFract * totalrows); vac_update_relstats(Irel[ind], RelationGetNumberOfBlocks(Irel[ind]), - totalindexrows, false, InvalidTransactionId); + totalindexrows, + false, + InvalidTransactionId, + in_outer_xact); } } diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index d6e99eeff8..41ca15c662 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -199,6 +199,8 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast, */ if (use_own_xacts) { + Assert(!in_outer_xact); + /* ActiveSnapshot is not set by autovacuum */ if (ActiveSnapshotSet()) PopActiveSnapshot(); @@ -241,7 +243,7 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast, PushActiveSnapshot(GetTransactionSnapshot()); } - analyze_rel(relid, vacstmt, vac_strategy); + analyze_rel(relid, vacstmt, in_outer_xact, vac_strategy); if (use_own_xacts) { @@ -555,20 +557,21 @@ vac_estimate_reltuples(Relation relation, bool is_analyze, * DDL flags such as relhasindex, by clearing them if no longer correct. * It's safe to do this in VACUUM, which can't run in parallel with * CREATE INDEX/RULE/TRIGGER and can't be part of a transaction block. - * However, it's *not* safe to do it in an ANALYZE that's within a - * transaction block, because for example the current transaction might + * However, it's *not* safe to do it in an ANALYZE that's within an + * outer transaction, because for example the current transaction might * have dropped the last index; then we'd think relhasindex should be * cleared, but if the transaction later rolls back this would be wrong. - * So we refrain from updating the DDL flags if we're inside a - * transaction block. This is OK since postponing the flag maintenance - * is always allowable. + * So we refrain from updating the DDL flags if we're inside an outer + * transaction. This is OK since postponing the flag maintenance is + * always allowable. * * This routine is shared by VACUUM and ANALYZE. */ void vac_update_relstats(Relation relation, BlockNumber num_pages, double num_tuples, - bool hasindex, TransactionId frozenxid) + bool hasindex, TransactionId frozenxid, + bool in_outer_xact) { Oid relid = RelationGetRelid(relation); Relation rd; @@ -599,9 +602,9 @@ vac_update_relstats(Relation relation, dirty = true; } - /* Apply DDL updates, but not inside a transaction block (see above) */ + /* Apply DDL updates, but not inside an outer transaction (see above) */ - if (!IsTransactionBlock()) + if (!in_outer_xact) { /* * If we didn't find any indexes, reset relhasindex. diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c index 57daca5018..25278025b0 100644 --- a/src/backend/commands/vacuumlazy.c +++ b/src/backend/commands/vacuumlazy.c @@ -266,7 +266,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt, vac_update_relstats(onerel, new_rel_pages, new_rel_tuples, vacrelstats->hasindex, - new_frozen_xid); + new_frozen_xid, + false); /* report results to the stats collector, too */ pgstat_report_vacuum(RelationGetRelid(onerel), @@ -1091,7 +1092,9 @@ lazy_cleanup_index(Relation indrel, if (!stats->estimated_count) vac_update_relstats(indrel, stats->num_pages, stats->num_index_tuples, - false, InvalidTransactionId); + false, + InvalidTransactionId, + false); ereport(elevel, (errmsg("index \"%s\" now contains %.0f row versions in %u pages", diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 9ec9799268..317dbb5ebf 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -150,7 +150,8 @@ extern void vac_update_relstats(Relation relation, BlockNumber num_pages, double num_tuples, bool hasindex, - TransactionId frozenxid); + TransactionId frozenxid, + bool in_outer_xact); extern void vacuum_set_xid_limits(int freeze_min_age, int freeze_table_age, bool sharedRel, TransactionId *oldestXmin, @@ -165,6 +166,6 @@ extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt, /* in commands/analyze.c */ extern void analyze_rel(Oid relid, VacuumStmt *vacstmt, - BufferAccessStrategy bstrategy); + bool in_outer_xact, BufferAccessStrategy bstrategy); #endif /* VACUUM_H */