From 9ab2f39529155cee6a700ca6e8de1dd095ae9ed6 Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 6 Jul 2026 16:42:19 -0700 Subject: [PATCH] Report the unresolvable chunk when the gc mark phase fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bare "gc mark phase failed" gives a field report nothing to act on. Track each queued hash's provenance through the mark walk and name the missing chunk, its parent hash, the edge source (refs table, branch tip/working set, tag, pending, session, or a child edge inside a stored chunk), the parent's chunk type — with level/item count for prolly nodes — the child ordinal, and the result code. This is the diagnostic layer from the closed #1556 without its test-build counters and stress gate; its field capture is what pinpointed the #1547 misclassification. Co-Authored-By: Claude Fable 5 --- src/doltlite_gc.c | 206 ++++++++++++++++++++++++++++++++++++++------ test/doltlite_gc.sh | 17 ++++ 2 files changed, 197 insertions(+), 26 deletions(-) diff --git a/src/doltlite_gc.c b/src/doltlite_gc.c index 3870293878..83d19a5ce1 100644 --- a/src/doltlite_gc.c +++ b/src/doltlite_gc.c @@ -16,16 +16,64 @@ extern void csSerializeManifest(const ChunkStore *cs, u8 *aBuf); #include "doltlite_internal.h" typedef struct GcQueue GcQueue; +typedef struct GcQueueItem GcQueueItem; +struct GcQueueItem { + ProllyHash hash; + ProllyHash parent; + const char *zSource; + const char *zParentType; + int iParentLevel; + int nParentItems; + int iChild; +}; struct GcQueue { - ProllyHash *aItems; + GcQueueItem *aItems; int nItems; int nAlloc; int iHead; }; +typedef struct GcChildCtx GcChildCtx; +struct GcChildCtx { + GcQueue *q; + ProllyHash parent; + const char *zSource; + const char *zParentType; + int iParentLevel; + int nParentItems; + int iNextChild; +}; + +typedef struct GcMarkTrace GcMarkTrace; +struct GcMarkTrace { + int hasMissing; + int rc; + ProllyHash missing; + ProllyHash parent; + const char *zSource; + const char *zParentType; + int iParentLevel; + int nParentItems; + int iChild; +}; + +static const char *gcChunkTypeName(DoltliteChunkType type){ + switch( type ){ + case CHUNK_COMMIT: return "commit"; + case CHUNK_PROLLY_NODE: return "prolly-node"; + case CHUNK_CATALOG: return "catalog"; + case CHUNK_WORKING_SET: return "working-set"; + case CHUNK_REFS: return "refs"; + case CHUNK_CONFLICTS: return "conflicts"; + case CHUNK_CONSTRAINT_VIOLATIONS: return "constraint-violations"; + case CHUNK_UNKNOWN: + default: return "unknown"; + } +} + static int gcQueueInit(GcQueue *q){ q->nAlloc = 256; - q->aItems = sqlite3_malloc(q->nAlloc * sizeof(ProllyHash)); + q->aItems = sqlite3_malloc(q->nAlloc * sizeof(GcQueueItem)); if( !q->aItems ) return SQLITE_NOMEM; q->nItems = 0; q->iHead = 0; @@ -37,30 +85,52 @@ static void gcQueueFree(GcQueue *q){ memset(q, 0, sizeof(*q)); } -static int gcQueuePush(GcQueue *q, const ProllyHash *h){ +static int gcQueuePush( + GcQueue *q, + const ProllyHash *h, + const ProllyHash *pParent, + const char *zSource, + const char *zParentType, + int iParentLevel, + int nParentItems, + int iChild +){ if( prollyHashIsEmpty(h) ) return SQLITE_OK; if( q->nItems >= q->nAlloc ){ int newAlloc = q->nAlloc * 2; - ProllyHash *aNew = sqlite3_realloc(q->aItems, newAlloc * sizeof(ProllyHash)); + GcQueueItem *aNew = sqlite3_realloc(q->aItems, newAlloc * sizeof(GcQueueItem)); if( !aNew ) return SQLITE_NOMEM; q->aItems = aNew; q->nAlloc = newAlloc; } - memcpy(&q->aItems[q->nItems], h, sizeof(ProllyHash)); + memcpy(&q->aItems[q->nItems].hash, h, sizeof(ProllyHash)); + if( pParent ){ + memcpy(&q->aItems[q->nItems].parent, pParent, sizeof(ProllyHash)); + }else{ + memset(&q->aItems[q->nItems].parent, 0, sizeof(ProllyHash)); + } + q->aItems[q->nItems].zSource = zSource; + q->aItems[q->nItems].zParentType = zParentType; + q->aItems[q->nItems].iParentLevel = iParentLevel; + q->aItems[q->nItems].nParentItems = nParentItems; + q->aItems[q->nItems].iChild = iChild; q->nItems++; return SQLITE_OK; } -static int gcQueuePop(GcQueue *q, ProllyHash *h){ +static int gcQueuePop(GcQueue *q, GcQueueItem *pItem){ if( q->iHead >= q->nItems ) return 0; - memcpy(h, &q->aItems[q->iHead], sizeof(ProllyHash)); + *pItem = q->aItems[q->iHead]; q->iHead++; return 1; } static int gcChildCb(void *ctx, const ProllyHash *pHash){ - GcQueue *q = (GcQueue*)ctx; - return gcQueuePush(q, pHash); + GcChildCtx *p = (GcChildCtx*)ctx; + int iChild = p->iNextChild++; + return gcQueuePush(p->q, pHash, &p->parent, p->zSource, + p->zParentType, p->iParentLevel, p->nParentItems, + iChild); } #ifdef DOLTLITE_PROLLY_CHECK @@ -97,23 +167,31 @@ static void gcVerifySessionResolvable(sqlite3 *db, ChunkStore *cs){ static int gcMarkReachable( sqlite3 *db, ChunkStore *cs, - ProllyHashSet *marked + ProllyHashSet *marked, + GcMarkTrace *pTrace ){ GcQueue queue; - ProllyHash current; + GcQueueItem current; + GcChildCtx seedCtx; int rc, i; + memset(pTrace, 0, sizeof(*pTrace)); rc = gcQueueInit(&queue); if( rc!=SQLITE_OK ) return rc; - rc = gcQueuePush(&queue, refsTableGetHash(&cs->refs)); + rc = gcQueuePush(&queue, refsTableGetHash(&cs->refs), 0, "refs-table", + 0, -1, -1, -1); { int nBr; const BranchRef *aBr; refsTableGetBranches(&cs->refs, &nBr, &aBr); for(i=0; rc==SQLITE_OK && irefs, &nTg, &aTg); for(i=0; rc==SQLITE_OK && istaging, &nPend, &aPend); for(i=0; rc==SQLITE_OK && ihasMissing = 1; + pTrace->rc = rc; + memcpy(&pTrace->missing, ¤t.hash, sizeof(ProllyHash)); + memcpy(&pTrace->parent, ¤t.parent, sizeof(ProllyHash)); + pTrace->zSource = current.zSource; + pTrace->zParentType = current.zParentType; + pTrace->iParentLevel = current.iParentLevel; + pTrace->nParentItems = current.nParentItems; + pTrace->iChild = current.iChild; + } break; } - rc = doltliteEnumerateChunkChildren(data, nData, gcChildCb, &queue); + childCtx.q = &queue; + childCtx.parent = current.hash; + childCtx.zSource = "child"; + childCtx.zParentType = gcChunkTypeName(doltliteClassifyChunk(data, nData)); + childCtx.iParentLevel = -1; + childCtx.nParentItems = -1; + childCtx.iNextChild = 0; + if( strcmp(childCtx.zParentType, "prolly-node")==0 ){ + ProllyNode node; + if( prollyNodeParse(&node, data, nData)==SQLITE_OK ){ + childCtx.iParentLevel = node.level; + childCtx.nParentItems = (int)node.nItems; + } + } + rc = doltliteEnumerateChunkChildren(data, nData, gcChildCb, &childCtx); sqlite3_free(data); if( rc!=SQLITE_OK ) break; @@ -167,6 +276,41 @@ static int gcMarkReachable( return rc; } +static void gcFormatMarkFailure( + char *zBuf, + int nBuf, + const GcMarkTrace *pTrace +){ + char zMissing[PROLLY_HASH_SIZE*2+1]; + char zParent[PROLLY_HASH_SIZE*2+1]; + if( !zBuf || nBuf<=0 || !pTrace || !pTrace->hasMissing ){ + return; + } + doltliteHashToHex(&pTrace->missing, zMissing); + if( prollyHashIsEmpty(&pTrace->parent) ){ + sqlite3_snprintf(nBuf, zBuf, + "gc mark phase failed: missing chunk %s source=%s rc=%d", + zMissing, pTrace->zSource ? pTrace->zSource : "unknown", pTrace->rc); + }else{ + doltliteHashToHex(&pTrace->parent, zParent); + if( pTrace->zParentType && strcmp(pTrace->zParentType, "prolly-node")==0 ){ + sqlite3_snprintf(nBuf, zBuf, + "gc mark phase failed: missing chunk %s parent=%s source=%s " + "parent_type=%s parent_level=%d parent_items=%d child_index=%d rc=%d", + zMissing, zParent, pTrace->zSource ? pTrace->zSource : "unknown", + pTrace->zParentType, pTrace->iParentLevel, pTrace->nParentItems, + pTrace->iChild, pTrace->rc); + }else{ + sqlite3_snprintf(nBuf, zBuf, + "gc mark phase failed: missing chunk %s parent=%s source=%s " + "parent_type=%s child_index=%d rc=%d", + zMissing, zParent, pTrace->zSource ? pTrace->zSource : "unknown", + pTrace->zParentType ? pTrace->zParentType : "unknown", + pTrace->iChild, pTrace->rc); + } + } +} + static int gcAppendMarkedChunk( ChunkStore *cs, const ProllyHash *pHash, @@ -666,10 +810,13 @@ static int gcRun( int *pnKept, int *pnRemoved, const char **pzPhase, + char *zPhaseBuf, + int nPhaseBuf, int bRequireExclusive, int bForceRefresh ){ ProllyHashSet marked; + GcMarkTrace markTrace; int rc; *pnKept = 0; @@ -701,11 +848,16 @@ static int gcRun( return rc; } - rc = gcMarkReachable(db, cs, &marked); + rc = gcMarkReachable(db, cs, &marked, &markTrace); if( rc!=SQLITE_OK ){ prollyHashSetFree(&marked); chunkStoreUnlock(cs); - *pzPhase = "gc mark phase failed"; + if( markTrace.hasMissing && zPhaseBuf && nPhaseBuf>0 ){ + gcFormatMarkFailure(zPhaseBuf, nPhaseBuf, &markTrace); + *pzPhase = zPhaseBuf; + }else{ + *pzPhase = "gc mark phase failed"; + } return rc; } @@ -729,6 +881,7 @@ static void doltliteGcFunc( int nKept = 0, nRemoved = 0; int rc; const char *zPhase = 0; + char zPhaseBuf[256]; char result[128]; (void)argc; @@ -744,7 +897,8 @@ static void doltliteGcFunc( return; } - rc = gcRun(db, cs, &nKept, &nRemoved, &zPhase, 1, 1); + rc = gcRun(db, cs, &nKept, &nRemoved, &zPhase, zPhaseBuf, + sizeof(zPhaseBuf), 1, 1); if( rc!=SQLITE_OK ){ if( rc==SQLITE_BUSY ){ sqlite3_result_error(context, @@ -774,7 +928,7 @@ int doltliteGcCompactWithPhase(sqlite3 *db, const char **pzPhase){ return SQLITE_OK; } - return gcRun(db, cs, &nKept, &nRemoved, pzPhase, 0, 0); + return gcRun(db, cs, &nKept, &nRemoved, pzPhase, 0, 0, 0, 0); } int doltliteGcCompact(sqlite3 *db){ diff --git a/test/doltlite_gc.sh b/test/doltlite_gc.sh index 3c0bbcca92..75c0e74ceb 100644 --- a/test/doltlite_gc.sh +++ b/test/doltlite_gc.sh @@ -307,4 +307,21 @@ run_test_match "gc_taghist_diff" \ db_rm "$DB" +# A mark failure must name the unresolvable chunk (hash, source, rc), not +# just "gc mark phase failed" — that hash is what makes a field report +# actionable. Flipping the first WAL record's tag byte makes the chunks +# behind it unreachable while the store still opens. +DB=/tmp/test_gc_mark_diag_$$.db; db_rm "$DB" +echo "CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT); +INSERT INTO t VALUES(1,'a'); +SELECT dolt_commit('-A','-m','seed');" | $DOLTLITE "$DB" > /dev/null 2>&1 +printf '\xff' | dd of="$DB" bs=1 seek=168 conv=notrunc 2>/dev/null + +run_test_match "gc_mark_failure_names_missing_chunk" \ + "SELECT dolt_gc();" \ + "missing chunk [0-9a-f]{40}.*source=.*rc=" \ + "$DB" + +db_rm "$DB" + dltest_finish