diff --git a/__test__/mirror-cleanup.test.ts b/__test__/mirror-cleanup.test.ts index fd3935c..b8b9469 100644 --- a/__test__/mirror-cleanup.test.ts +++ b/__test__/mirror-cleanup.test.ts @@ -107,10 +107,10 @@ describe('cleanup commit decision', () => { '-l', '-n', '--geometric=2', - '--write-midx', 'repack.writeBitmaps=false' ]) ) + expect(repack).not.toContain('--write-midx') expect(commands().some(c => c.includes('gc'))).toBe(false) expect(mockCommitStickyDisk).toHaveBeenCalledWith( expect.objectContaining({shouldCommit: true, vmHydratedGitMirror: true}) diff --git a/__test__/mirror-maintenance-git.test.ts b/__test__/mirror-maintenance-git.test.ts index d51fd11..110f6fd 100644 --- a/__test__/mirror-maintenance-git.test.ts +++ b/__test__/mirror-maintenance-git.test.ts @@ -206,12 +206,41 @@ describe('runMirrorMaintenance (real git)', () => { expect(remaining).toContain(basePack) expect(remaining.length).toBeLessThanOrEqual(3) expect(looseObjects(mirror)).toBe(0) - expect(fs.existsSync(path.join(packDir, 'multi-pack-index'))).toBe(true) + expect(fs.existsSync(path.join(packDir, 'multi-pack-index'))).toBe(false) expect(fs.existsSync(path.join(mirror, 'packed-refs'))).toBe(true) fsck(mirror) refsResolve(mirror) }) + it('removes a multi-pack-index left by an earlier version and still rolls up the packs it lists', async () => { + const {mirror, basePack} = buildMirror(root) + const packDir = path.join(mirror, 'objects', 'pack') + git(mirror, 'multi-pack-index', 'write') + expect(fs.existsSync(path.join(packDir, 'multi-pack-index'))).toBe(true) + const before = packs(mirror) + + const result = await blacksmithCache.runMirrorMaintenance(mirror, { + timeoutSecs: 60, + keepBytes: KEEP_BYTES + }) + expect(result).toMatchObject({success: true, timedOut: false}) + expect(result.skipped).toBeUndefined() + + expect(fs.existsSync(path.join(packDir, 'multi-pack-index'))).toBe(false) + const remaining = packs(mirror) + expect(remaining).toContain(basePack) + expect(remaining.length).toBeLessThanOrEqual(3) + // Every small pack the stale index listed was folded, not skipped. + for (const p of before) { + if (p !== basePack) { + expect(remaining).not.toContain(p) + } + } + expect(looseObjects(mirror)).toBe(0) + fsck(mirror) + refsResolve(mirror) + }) + it('is a no-op on an already maintained mirror', async () => { const {mirror} = buildMirror(root) await blacksmithCache.runMirrorMaintenance(mirror, { @@ -482,7 +511,7 @@ describe('runMirrorMaintenance (real git)', () => { ) .trim() ).toBe('20000') - expect(fs.existsSync(path.join(packDir, 'multi-pack-index'))).toBe(true) + expect(fs.existsSync(path.join(packDir, 'multi-pack-index'))).toBe(false) // The graph was rebuilt without the pruned commit. expect(blacksmithCache.hasCommitGraph(mirror)).toBe(true) git(mirror, 'commit-graph', 'verify') diff --git a/dist/index.js b/dist/index.js index b22f509..83c036c 100644 --- a/dist/index.js +++ b/dist/index.js @@ -1449,6 +1449,19 @@ function writeCommitGraph(mirrorPath_1) { } }); } +function removeMultiPackIndex(mirrorPath) { + return __awaiter(this, void 0, void 0, function* () { + const target = path.join(mirrorPath, 'objects', 'pack', 'multi-pack-index'); + try { + yield fs.promises.rm(target, { force: true }); + return true; + } + catch (error) { + core.warning(`[git-mirror] Failed to remove ${target}: ${error}`); + return false; + } + }); +} function removeCommitGraph(mirrorPath) { return __awaiter(this, void 0, void 0, function* () { const info = path.join(mirrorPath, 'objects', 'info'); @@ -1671,8 +1684,14 @@ function reclaimDue(mirrorPath, now, intervalMs) { * chosen so their combined size stays under that bound as well (see * markKeepPacks). A rolled-up pack that grows past the threshold simply * becomes another kept pack, so the cost of a single run is bounded by the - * threshold, never by the size of the repository. The multi-pack-index - * keeps lookups fast across the kept packs. + * threshold, never by the size of the repository. + * + * No multi-pack-index is written: git rewrites it whole on every write, + * reading every pack's .idx and writing about as many bytes again, which + * on a mirror with millions of objects costs seconds of cold I/O per job - + * more than the roll-up itself - while lookups across the few packs the + * geometric repack leaves are fast without it. An index left by an earlier + * version is removed rather than left to go stale. * * Kept packs never lose objects, so history that becomes unreachable stays * on disk. Once per MAINTENANCE_RECLAIM_INTERVAL_MS the run instead lifts @@ -1717,14 +1736,14 @@ function runMirrorMaintenance(mirrorPath_1) { const { kept } = yield markKeepPacks(mirrorPath, Number.MAX_SAFE_INTEGER); yield removeKeepFiles(mirrorPath, kept); label = 'Reclaim'; - repackArgs = ['-a', '-d', '-l', '-n', '--write-midx']; + repackArgs = ['-a', '-d', '-l', '-n']; core.info(`[git-mirror] Running reclaim maintenance (timeout: ${budgetSecs}s, ${kept.length} kept pack(s) released)`); } else { const selection = yield markKeepPacks(mirrorPath, keepBytes); deferred = selection.deferred; label = 'Incremental'; - repackArgs = ['-d', '-l', '-n', '--geometric=2', '--write-midx']; + repackArgs = ['-d', '-l', '-n', '--geometric=2']; core.info(`[git-mirror] Running incremental maintenance (timeout: ${budgetSecs}s, ${selection.kept.length} kept pack(s), ${deferred.length} deferred)`); } // Pack size delta across the repack approximates the bytes reclaimed by @@ -1744,6 +1763,9 @@ function runMirrorMaintenance(mirrorPath_1) { }; }); try { + if (!(yield removeMultiPackIndex(mirrorPath))) { + return yield fail(false, 'could not remove the multi-pack-index'); + } const result = yield exec.getExecOutput('timeout', [ String(remainingSecs()), 'git', diff --git a/src/blacksmith-cache.ts b/src/blacksmith-cache.ts index fcaa55e..c19d29b 100644 --- a/src/blacksmith-cache.ts +++ b/src/blacksmith-cache.ts @@ -1724,6 +1724,17 @@ async function writeCommitGraph( } } +async function removeMultiPackIndex(mirrorPath: string): Promise { + const target = path.join(mirrorPath, 'objects', 'pack', 'multi-pack-index') + try { + await fs.promises.rm(target, {force: true}) + return true + } catch (error) { + core.warning(`[git-mirror] Failed to remove ${target}: ${error}`) + return false + } +} + async function removeCommitGraph(mirrorPath: string): Promise { const info = path.join(mirrorPath, 'objects', 'info') for (const target of [ @@ -1982,8 +1993,14 @@ export interface MaintenanceOptions { * chosen so their combined size stays under that bound as well (see * markKeepPacks). A rolled-up pack that grows past the threshold simply * becomes another kept pack, so the cost of a single run is bounded by the - * threshold, never by the size of the repository. The multi-pack-index - * keeps lookups fast across the kept packs. + * threshold, never by the size of the repository. + * + * No multi-pack-index is written: git rewrites it whole on every write, + * reading every pack's .idx and writing about as many bytes again, which + * on a mirror with millions of objects costs seconds of cold I/O per job - + * more than the roll-up itself - while lookups across the few packs the + * geometric repack leaves are fast without it. An index left by an earlier + * version is removed rather than left to go stale. * * Kept packs never lose objects, so history that becomes unreachable stays * on disk. Once per MAINTENANCE_RECLAIM_INTERVAL_MS the run instead lifts @@ -2035,7 +2052,7 @@ export async function runMirrorMaintenance( const {kept} = await markKeepPacks(mirrorPath, Number.MAX_SAFE_INTEGER) await removeKeepFiles(mirrorPath, kept) label = 'Reclaim' - repackArgs = ['-a', '-d', '-l', '-n', '--write-midx'] + repackArgs = ['-a', '-d', '-l', '-n'] core.info( `[git-mirror] Running reclaim maintenance (timeout: ${budgetSecs}s, ${kept.length} kept pack(s) released)` ) @@ -2043,7 +2060,7 @@ export async function runMirrorMaintenance( const selection = await markKeepPacks(mirrorPath, keepBytes) deferred = selection.deferred label = 'Incremental' - repackArgs = ['-d', '-l', '-n', '--geometric=2', '--write-midx'] + repackArgs = ['-d', '-l', '-n', '--geometric=2'] core.info( `[git-mirror] Running incremental maintenance (timeout: ${budgetSecs}s, ${selection.kept.length} kept pack(s), ${deferred.length} deferred)` ) @@ -2075,6 +2092,9 @@ export async function runMirrorMaintenance( } try { + if (!(await removeMultiPackIndex(mirrorPath))) { + return await fail(false, 'could not remove the multi-pack-index') + } const result = await exec.getExecOutput( 'timeout', [