Commit 3b2787e1 authored by Robert Haas's avatar Robert Haas

Fix broken cleanup interlock for GIN pending list.

The pending list must (for correctness) always be cleaned up by vacuum, and
should (for the avoidance of surprising behavior) always be cleaned up
by an explicit call to gin_clean_pending_list, but cleanup is optional
when inserting.  The old logic got this backward: cleanup was forced
if (stats == NULL), but that's going to be *false* when vacuuming and
*true* for inserts.

Masahiko Sawada, reviewed by me.

Discussion: http://postgr.es/m/CAD21AoBLUSyiYKnTYtSAbC+F=XDjiaBrOUEGK+zUXdQ8owfPKw@mail.gmail.com
parent 6b2cd278
...@@ -450,8 +450,12 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector) ...@@ -450,8 +450,12 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
END_CRIT_SECTION(); END_CRIT_SECTION();
/*
* Since it could contend with concurrent cleanup process we cleanup
* pending list not forcibly.
*/
if (needCleanup) if (needCleanup)
ginInsertCleanup(ginstate, false, true, NULL); ginInsertCleanup(ginstate, false, true, false, NULL);
} }
/* /*
...@@ -748,7 +752,8 @@ processPendingPage(BuildAccumulator *accum, KeyArray *ka, ...@@ -748,7 +752,8 @@ processPendingPage(BuildAccumulator *accum, KeyArray *ka,
*/ */
void void
ginInsertCleanup(GinState *ginstate, bool full_clean, ginInsertCleanup(GinState *ginstate, bool full_clean,
bool fill_fsm, IndexBulkDeleteResult *stats) bool fill_fsm, bool forceCleanup,
IndexBulkDeleteResult *stats)
{ {
Relation index = ginstate->index; Relation index = ginstate->index;
Buffer metabuffer, Buffer metabuffer,
...@@ -765,7 +770,6 @@ ginInsertCleanup(GinState *ginstate, bool full_clean, ...@@ -765,7 +770,6 @@ ginInsertCleanup(GinState *ginstate, bool full_clean,
bool cleanupFinish = false; bool cleanupFinish = false;
bool fsm_vac = false; bool fsm_vac = false;
Size workMemory; Size workMemory;
bool inVacuum = (stats == NULL);
/* /*
* We would like to prevent concurrent cleanup process. For that we will * We would like to prevent concurrent cleanup process. For that we will
...@@ -774,7 +778,7 @@ ginInsertCleanup(GinState *ginstate, bool full_clean, ...@@ -774,7 +778,7 @@ ginInsertCleanup(GinState *ginstate, bool full_clean,
* insertion into pending list * insertion into pending list
*/ */
if (inVacuum) if (forceCleanup)
{ {
/* /*
* We are called from [auto]vacuum/analyze or gin_clean_pending_list() * We are called from [auto]vacuum/analyze or gin_clean_pending_list()
...@@ -1036,7 +1040,7 @@ gin_clean_pending_list(PG_FUNCTION_ARGS) ...@@ -1036,7 +1040,7 @@ gin_clean_pending_list(PG_FUNCTION_ARGS)
memset(&stats, 0, sizeof(stats)); memset(&stats, 0, sizeof(stats));
initGinState(&ginstate, indexRel); initGinState(&ginstate, indexRel);
ginInsertCleanup(&ginstate, true, true, &stats); ginInsertCleanup(&ginstate, true, true, true, &stats);
index_close(indexRel, AccessShareLock); index_close(indexRel, AccessShareLock);
......
...@@ -570,7 +570,7 @@ ginbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, ...@@ -570,7 +570,7 @@ ginbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
* and cleanup any pending inserts * and cleanup any pending inserts
*/ */
ginInsertCleanup(&gvs.ginstate, !IsAutoVacuumWorkerProcess(), ginInsertCleanup(&gvs.ginstate, !IsAutoVacuumWorkerProcess(),
false, stats); false, true, stats);
} }
/* we'll re-count the tuples each time */ /* we'll re-count the tuples each time */
...@@ -683,7 +683,7 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) ...@@ -683,7 +683,7 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
if (IsAutoVacuumWorkerProcess()) if (IsAutoVacuumWorkerProcess())
{ {
initGinState(&ginstate, index); initGinState(&ginstate, index);
ginInsertCleanup(&ginstate, false, true, stats); ginInsertCleanup(&ginstate, false, true, true, stats);
} }
return stats; return stats;
} }
...@@ -697,7 +697,7 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats) ...@@ -697,7 +697,7 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult)); stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
initGinState(&ginstate, index); initGinState(&ginstate, index);
ginInsertCleanup(&ginstate, !IsAutoVacuumWorkerProcess(), ginInsertCleanup(&ginstate, !IsAutoVacuumWorkerProcess(),
false, stats); false, true, stats);
} }
memset(&idxStat, 0, sizeof(idxStat)); memset(&idxStat, 0, sizeof(idxStat));
......
...@@ -439,7 +439,7 @@ extern void ginHeapTupleFastCollect(GinState *ginstate, ...@@ -439,7 +439,7 @@ extern void ginHeapTupleFastCollect(GinState *ginstate,
OffsetNumber attnum, Datum value, bool isNull, OffsetNumber attnum, Datum value, bool isNull,
ItemPointer ht_ctid); ItemPointer ht_ctid);
extern void ginInsertCleanup(GinState *ginstate, bool full_clean, extern void ginInsertCleanup(GinState *ginstate, bool full_clean,
bool fill_fsm, IndexBulkDeleteResult *stats); bool fill_fsm, bool forceCleanup, IndexBulkDeleteResult *stats);
/* ginpostinglist.c */ /* ginpostinglist.c */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment