Switched to TidStore for HNSW vacuuming for Postgres 17+

Co-authored-by: Bhagyesh Chaturvedi <bhagyeshc@google.com>

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## 0.8.3 (unreleased)
 
+- Fixed possible index corruption with HNSW vacuuming
 - Fixed performance regression with Hamming distance and Jaccard distance with Postgres 18
 
 ## 0.8.2 (2026-02-25)

src/hnsw.c

@@ -290,7 +290,11 @@ hnswhandler(PG_FUNCTION_ARGS)
 		.amcanparallel = false,
 		.amcanbuildparallel = true,
 		.amcaninclude = false,
+#if PG_VERSION_NUM >= 170000
+		.amusemaintenanceworkmem = true,
+#else
 		.amusemaintenanceworkmem = false,
+#endif
 		.amsummarizing = false,
 		.amparallelvacuumoptions = VACUUM_OPTION_PARALLEL_BULKDEL,
 		.amkeytype = InvalidOid,
@@ -351,7 +355,11 @@ hnswhandler(PG_FUNCTION_ARGS)
 	amroutine->amcanbuildparallel = true;
 #endif
 	amroutine->amcaninclude = false;
-	amroutine->amusemaintenanceworkmem = false; /* not used during VACUUM */
+#if PG_VERSION_NUM >= 170000
+	amroutine->amusemaintenanceworkmem = true;
+#else
+	amroutine->amusemaintenanceworkmem = false;
+#endif
 #if PG_VERSION_NUM >= 160000
 	amroutine->amsummarizing = false;
 #endif

src/hnsw.h

@@ -26,6 +26,12 @@
 typedef Pointer Item;
 #endif
 
+#if PG_VERSION_NUM >= 170000
+#define HnswTidStore TidStore
+#else
+#define HnswTidStore tidhash_hash
+#endif
+
 #define HNSW_MAX_DIM 2000
 #define HNSW_MAX_NNZ 1000
 
@@ -427,10 +433,11 @@ typedef struct HnswVacuumState
 	HnswSupport support;
 
 	/* Variables */
-	struct tidhash_hash *deleted;
+	struct HnswTidStore *deleted;
 	BufferAccessStrategy bas;
 	HnswNeighborTuple ntup;
 	HnswElementData highestPoint;
+	HnswElementData fallbackPoint;
 
 	/* Memory */
 	MemoryContext tmpCtx;

src/hnswutils.c

@@ -546,6 +546,9 @@ HnswLoadElementImpl(BlockNumber blkno, OffsetNumber offno, double *distance, Hns
 
 	Assert(HnswIsElementTuple(etup));
 
+	if (unlikely(etup->deleted))
+		elog(ERROR, "cannot load deleted element");
+
 	/* Calculate distance */
 	if (distance != NULL)
 	{

src/hnswvacuum.c

@@ -10,6 +10,12 @@
 #include "utils/memutils.h"
 #include "utils/rel.h"
 
+#if PG_VERSION_NUM >= 170000
+#include "access/tidstore.h"
+#include "miscadmin.h"
+#include "postmaster/autovacuum.h"
+#endif
+
 #if PG_VERSION_NUM >= 160000
 #include "varatt.h"
 #endif
@@ -22,9 +28,13 @@
  * Check if deleted list contains an index TID
  */
 static bool
-DeletedContains(tidhash_hash * deleted, ItemPointer indextid)
+DeletedContains(HnswTidStore * deleted, ItemPointer indextid)
 {
+#if PG_VERSION_NUM >= 170000
+	return TidStoreIsMember(deleted, indextid);
+#else
 	return tidhash_lookup(deleted, *indextid) != NULL;
+#endif
 }
 
 /*
@@ -37,17 +47,20 @@ RemoveHeapTids(HnswVacuumState * vacuumstate)
 {
 	BlockNumber blkno = HNSW_HEAD_BLKNO;
 	HnswElement highestPoint = &vacuumstate->highestPoint;
+	HnswElement fallbackPoint = &vacuumstate->fallbackPoint;
 	Relation	index = vacuumstate->index;
 	BufferAccessStrategy bas = vacuumstate->bas;
-	HnswElement entryPoint = HnswGetEntryPoint(vacuumstate->index);
 	IndexBulkDeleteResult *stats = vacuumstate->stats;
 
-	/* Store separately since highestPoint.level is uint8 */
+	/* Store separately since HnswElement level is uint8 */
 	int			highestLevel = -1;
+	int			fallbackLevel = -1;
 
-	/* Initialize highest point */
+	/* Initialize highest point and fallback point */
 	highestPoint->blkno = InvalidBlockNumber;
 	highestPoint->offno = InvalidOffsetNumber;
+	fallbackPoint->blkno = InvalidBlockNumber;
+	fallbackPoint->offno = InvalidOffsetNumber;
 
 	while (BlockNumberIsValid(blkno))
 	{
@@ -57,6 +70,10 @@ RemoveHeapTids(HnswVacuumState * vacuumstate)
 		OffsetNumber offno;
 		OffsetNumber maxoffno;
 		bool		updated = false;
+#if PG_VERSION_NUM >= 170000
+		OffsetNumber deletedoffs[MaxOffsetNumber];
+		int			ndeletedoffs = 0;
+#endif
 
 		vacuum_delay_point();
 
@@ -110,6 +127,9 @@ RemoveHeapTids(HnswVacuumState * vacuumstate)
 
 			if (!ItemPointerIsValid(&etup->heaptids[0]))
 			{
+#if PG_VERSION_NUM >= 170000
+				deletedoffs[ndeletedoffs++] = offno;
+#else
 				ItemPointerData ip;
 				bool		found;
 
@@ -118,17 +138,39 @@ RemoveHeapTids(HnswVacuumState * vacuumstate)
 
 				tidhash_insert(vacuumstate->deleted, ip, &found);
 				Assert(!found);
+#endif
 			}
-			else if (etup->level > highestLevel && !(entryPoint != NULL && blkno == entryPoint->blkno && offno == entryPoint->offno))
+			else if (etup->level > highestLevel)
 			{
-				/* Keep track of highest non-entry point */
+				if (BlockNumberIsValid(highestPoint->blkno))
+				{
+					/* Current highest point becomes fallback */
+					fallbackPoint->blkno = highestPoint->blkno;
+					fallbackPoint->offno = highestPoint->offno;
+					fallbackPoint->level = highestPoint->level;
+					fallbackLevel = highestLevel;
+				}
+
+				/* Keep track of highest point */
 				highestPoint->blkno = blkno;
 				highestPoint->offno = offno;
 				highestPoint->level = etup->level;
 				highestLevel = etup->level;
 			}
+			else if (etup->level > fallbackLevel)
+			{
+				/* Keep track of second highest point */
+				fallbackPoint->blkno = blkno;
+				fallbackPoint->offno = offno;
+				fallbackPoint->level = etup->level;
+				fallbackLevel = etup->level;
+			}
 		}
 
+#if PG_VERSION_NUM >= 170000
+		TidStoreSetBlockOffsets(vacuumstate->deleted, blkno, deletedoffs, ndeletedoffs);
+#endif
+
 		blkno = HnswPageGetOpaque(page)->nextblkno;
 
 		if (updated)
@@ -138,6 +180,10 @@ RemoveHeapTids(HnswVacuumState * vacuumstate)
 
 		UnlockReleaseBuffer(buf);
 	}
+
+#ifdef HNSW_MEMORY
+	elog(INFO, "memory: %zu KB", MemoryContextMemAllocated(CurrentMemoryContext, true) / 1024);
+#endif
 }
 
 /*
@@ -269,12 +315,27 @@ RepairGraphEntryPoint(HnswVacuumState * vacuumstate)
 		/* Get a shared lock */
 		LockPage(index, HNSW_UPDATE_LOCK, ShareLock);
 
-		/* Load element */
+		/* Get latest entry point */
-		HnswLoadElement(highestPoint, NULL, NULL, index, support, true, NULL);
+		entryPoint = HnswGetEntryPoint(index);
 
-		/* Repair if needed */
+		/* Use fallback point if highest point is entry point */
-		if (NeedsUpdated(vacuumstate, highestPoint))
+		if (entryPoint != NULL && entryPoint->blkno == highestPoint->blkno && entryPoint->offno == highestPoint->offno)
-			RepairGraphElement(vacuumstate, highestPoint, HnswGetEntryPoint(index));
+		{
+			highestPoint = &vacuumstate->fallbackPoint;
+
+			if (!BlockNumberIsValid(highestPoint->blkno))
+				highestPoint = NULL;
+		}
+
+		if (highestPoint != NULL)
+		{
+			/* Load element */
+			HnswLoadElement(highestPoint, NULL, NULL, index, support, true, NULL);
+
+			/* Repair if needed */
+			if (NeedsUpdated(vacuumstate, highestPoint))
+				RepairGraphElement(vacuumstate, highestPoint, entryPoint);
+		}
 
 		/* Release lock */
 		UnlockPage(index, HNSW_UPDATE_LOCK, ShareLock);
@@ -441,6 +502,99 @@ RepairGraph(HnswVacuumState * vacuumstate)
 		/* Reset memory context */
 		MemoryContextSwitchTo(oldCtx);
 		MemoryContextReset(vacuumstate->tmpCtx);
+
+#ifdef HNSW_VACUUM_PROGRESS
+		if (!BlockNumberIsValid(blkno) || ((blkno - HNSW_HEAD_BLKNO) % 1000 == 0 && blkno != HNSW_HEAD_BLKNO))
+		{
+			BlockNumber totalBlocks = RelationGetNumberOfBlocks(index);
+			BlockNumber currentBlocks = BlockNumberIsValid(blkno) ? blkno : totalBlocks;
+
+			elog(INFO, "hnsw vacuum progress: %.1f%%", 100.0 * currentBlocks / totalBlocks);
+		}
+#endif
+	}
+}
+
+/*
+ * Confirm graph was repaired
+ */
+static void
+ConfirmRepaired(HnswVacuumState * vacuumstate)
+{
+	BlockNumber blkno = HNSW_HEAD_BLKNO;
+	Relation	index = vacuumstate->index;
+	BufferAccessStrategy bas = vacuumstate->bas;
+
+	while (BlockNumberIsValid(blkno))
+	{
+		Buffer		buf;
+		Page		page;
+		OffsetNumber offno;
+		OffsetNumber maxoffno;
+
+		vacuum_delay_point();
+
+		buf = ReadBufferExtended(index, MAIN_FORKNUM, blkno, RBM_NORMAL, bas);
+		LockBuffer(buf, BUFFER_LOCK_SHARE);
+		page = BufferGetPage(buf);
+		maxoffno = PageGetMaxOffsetNumber(page);
+
+		/* Iterate over nodes */
+		for (offno = FirstOffsetNumber; offno <= maxoffno; offno = OffsetNumberNext(offno))
+		{
+			HnswElementTuple etup = (HnswElementTuple) PageGetItem(page, PageGetItemId(page, offno));
+			HnswNeighborTuple ntup;
+			Buffer		nbuf;
+			Page		npage;
+			BlockNumber neighborPage;
+			OffsetNumber neighborOffno;
+
+			/* Skip neighbor tuples */
+			if (!HnswIsElementTuple(etup))
+				continue;
+
+			/* Skip if being deleted */
+			if (!ItemPointerIsValid(&etup->heaptids[0]))
+				continue;
+
+			/* Get neighbor page */
+			neighborPage = ItemPointerGetBlockNumber(&etup->neighbortid);
+			neighborOffno = ItemPointerGetOffsetNumber(&etup->neighbortid);
+
+			if (neighborPage == blkno)
+			{
+				nbuf = buf;
+				npage = page;
+			}
+			else
+			{
+				nbuf = ReadBufferExtended(index, MAIN_FORKNUM, neighborPage, RBM_NORMAL, bas);
+				LockBuffer(nbuf, BUFFER_LOCK_SHARE);
+				npage = BufferGetPage(nbuf);
+			}
+
+			ntup = (HnswNeighborTuple) PageGetItem(npage, PageGetItemId(npage, neighborOffno));
+
+			/* Check neighbors */
+			for (int i = 0; i < ntup->count; i++)
+			{
+				ItemPointer indextid = &ntup->indextids[i];
+
+				if (!ItemPointerIsValid(indextid))
+					continue;
+
+				/* Check if in deleted list */
+				if (DeletedContains(vacuumstate->deleted, indextid))
+					elog(ERROR, "hnsw graph not repaired");
+			}
+
+			if (nbuf != buf)
+				UnlockReleaseBuffer(nbuf);
+		}
+
+		blkno = HnswPageGetOpaque(page)->nextblkno;
+
+		UnlockReleaseBuffer(buf);
 	}
 }
 
@@ -607,7 +761,11 @@ InitVacuumState(HnswVacuumState * vacuumstate, IndexVacuumInfo *info, IndexBulkD
 	HnswGetMetaPageInfo(index, &vacuumstate->m, NULL);
 
 	/* Create hash table */
+#if PG_VERSION_NUM >= 170000
+	vacuumstate->deleted = TidStoreCreateLocal((AmAutoVacuumWorkerProcess() && autovacuum_work_mem != -1) ? autovacuum_work_mem : maintenance_work_mem, true);
+#else
 	vacuumstate->deleted = tidhash_create(CurrentMemoryContext, 256, NULL);
+#endif
 }
 
 /*
@@ -616,7 +774,11 @@ InitVacuumState(HnswVacuumState * vacuumstate, IndexVacuumInfo *info, IndexBulkD
 static void
 FreeVacuumState(HnswVacuumState * vacuumstate)
 {
+#if PG_VERSION_NUM >= 170000
+	TidStoreDestroy(vacuumstate->deleted);
+#else
 	tidhash_destroy(vacuumstate->deleted);
+#endif
 	FreeAccessStrategy(vacuumstate->bas);
 	pfree(vacuumstate->ntup);
 	MemoryContextDelete(vacuumstate->tmpCtx);
@@ -639,7 +801,10 @@ hnswbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	/* Pass 2: Repair graph */
 	HnswBench("RepairGraph", RepairGraph(&vacuumstate));
 
-	/* Pass 3: Mark as deleted */
+	/* Pass 3: Confirm repaired */
+	HnswBench("ConfirmRepaired", ConfirmRepaired(&vacuumstate));
+
+	/* Pass 4: Mark as deleted */
 	HnswBench("MarkDeleted", MarkDeleted(&vacuumstate));
 
 	FreeVacuumState(&vacuumstate);

src/ivfbuild.c

@@ -399,6 +399,7 @@ InitBuildState(IvfflatBuildState * buildstate, Relation heap, Relation index, In
 
 	buildstate->slot = MakeSingleTupleTableSlot(buildstate->sortdesc, &TTSOpsVirtual);
 
+	/* TODO Ensure within maintenance_work_mem */
 	buildstate->centers = VectorArrayInit(buildstate->lists, buildstate->dimensions, buildstate->typeInfo->itemSize(buildstate->dimensions));
 	buildstate->listInfo = palloc(sizeof(ListInfo) * buildstate->lists);