Post step: stop writing the multi-pack-index during mirror maintenance

__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})

__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')

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',

src/blacksmith-cache.ts

@@ -1724,6 +1724,17 @@ async function writeCommitGraph(
   }
 }
 
+async function removeMultiPackIndex(mirrorPath: string): Promise<boolean> {
+  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<boolean> {
   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,15 +2052,15 @@ 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)`
     )
   } else {
     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',
       [