skip re-hashing the whole compose tree when git shows no change
This commit is contained in:
3 files changed
+109
-6
No files matched your search
@@ -376,6 +376,39 @@ export function buildNextManifest(newCommit: string, newFiles: Record<string, st
|
|||||||
return { commit: newCommit, files: { ...newFiles } };
|
return { commit: newCommit, files: { ...newFiles } };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Compare two commit refs by their 7-char short form (the DB stores 7-char,
|
||||||
|
* git returns 40-char); null/empty never matches. */
|
||||||
|
function sameCommit(a: string | null | undefined, b: string | null | undefined): boolean {
|
||||||
|
if (!a || !b) return false;
|
||||||
|
return a.substring(0, 7) === b.substring(0, 7);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether a sync can skip re-hashing the whole compose tree. Skipping is safe
|
||||||
|
* ONLY when the git diff proved no compose-dir change AND the diff was taken
|
||||||
|
* against the SAME commit the manifest was built at. The second condition is
|
||||||
|
* load-bearing: a bare Sync advances the diff baseline (lastCommit) without
|
||||||
|
* persisting the manifest (only a deploy does), so `changedInComposeDir===false`
|
||||||
|
* alone does NOT imply the on-disk tree still matches the manifest. When the
|
||||||
|
* baselines match and a prior manifest exists, nothing was added/modified/
|
||||||
|
* deleted, so the manifest carries forward verbatim and the unconditional
|
||||||
|
* full re-hash (which blocks the event loop on a large tree) is skipped. Any
|
||||||
|
* mismatch, undefined change state (new clone / diff failed), or empty manifest
|
||||||
|
* falls back to a full re-hash - the safe default that self-corrects drift.
|
||||||
|
*/
|
||||||
|
export function canSkipTreeRehash(
|
||||||
|
changedInComposeDir: boolean | undefined,
|
||||||
|
manifestSize: number,
|
||||||
|
manifestCommit: string | null | undefined,
|
||||||
|
diffBaselineCommit: string | null | undefined
|
||||||
|
): boolean {
|
||||||
|
return (
|
||||||
|
changedInComposeDir === false &&
|
||||||
|
manifestSize > 0 &&
|
||||||
|
sameCommit(manifestCommit, diffBaselineCommit)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// =============================================================================
|
// =============================================================================
|
||||||
// Sync summary (per-file status table)
|
// Sync summary (per-file status table)
|
||||||
// =============================================================================
|
// =============================================================================
|
||||||
|
|||||||
+37
-6
@@ -34,6 +34,7 @@ import {
|
|||||||
hashDirFiles,
|
hashDirFiles,
|
||||||
computeDeletions,
|
computeDeletions,
|
||||||
buildNextManifest,
|
buildNextManifest,
|
||||||
|
canSkipTreeRehash,
|
||||||
buildSyncChangeSummary,
|
buildSyncChangeSummary,
|
||||||
formatChangeTable,
|
formatChangeTable,
|
||||||
skipReasonMessage,
|
skipReasonMessage,
|
||||||
@@ -470,13 +471,34 @@ async function computeSyncDeletionPlan(options: {
|
|||||||
composeDir: string; // absolute path inside the clone
|
composeDir: string; // absolute path inside the clone
|
||||||
composeFileName: string | undefined; // compose file relative to composeDir
|
composeFileName: string | undefined; // compose file relative to composeDir
|
||||||
rawManifest: string | null | undefined;
|
rawManifest: string | null | undefined;
|
||||||
|
// When the caller's git diff proved NOTHING changed in the compose dir between
|
||||||
|
// the deployed commit and the new clone, hashing the whole tree is pure waste:
|
||||||
|
// no file changed, so none was deleted. Large trees (tens of thousands of files)
|
||||||
|
// otherwise block the event loop re-hashing unchanged files on every deploy.
|
||||||
|
// undefined = unknown -> hash (safe default; e.g. new clone / diff failed).
|
||||||
|
changedInComposeDir?: boolean;
|
||||||
|
// The commit the git diff was taken against. Skipping is only safe when this
|
||||||
|
// equals the manifest's own commit; otherwise the diff baseline has drifted
|
||||||
|
// from the manifest (e.g. a bare Sync advanced lastCommit without persisting
|
||||||
|
// the manifest) and a full re-hash is required to stay correct.
|
||||||
|
diffBaselineCommit?: string | null;
|
||||||
}): Promise<{ plan: DeletionPlan; newFiles: Record<string, string>; previousManifest: SyncManifest }> {
|
}): Promise<{ plan: DeletionPlan; newFiles: Record<string, string>; previousManifest: SyncManifest }> {
|
||||||
const { logPrefix, composeDir, composeFileName, rawManifest } = options;
|
const { logPrefix, composeDir, composeFileName, rawManifest, changedInComposeDir, diffBaselineCommit } = options;
|
||||||
|
|
||||||
const previousManifest = parseManifest(rawManifest);
|
const previousManifest = parseManifest(rawManifest);
|
||||||
|
const manifestSize = Object.keys(previousManifest.files).length;
|
||||||
|
|
||||||
|
// Fast path: git proved no compose-dir file changed, the diff baseline matches
|
||||||
|
// the manifest's commit, and we have a prior manifest. Nothing can have been
|
||||||
|
// added/modified/deleted, so carry the manifest forward verbatim instead of
|
||||||
|
// re-hashing the entire tree.
|
||||||
|
if (canSkipTreeRehash(changedInComposeDir, manifestSize, previousManifest.commit, diffBaselineCommit)) {
|
||||||
|
console.log(`${logPrefix} Deletion sync: no compose-dir changes, skipping full re-hash of ${manifestSize} file(s)`);
|
||||||
|
return { plan: { toDelete: [], skipped: [] }, newFiles: { ...previousManifest.files }, previousManifest };
|
||||||
|
}
|
||||||
|
|
||||||
const newFiles = hashDirFiles(composeDir);
|
const newFiles = hashDirFiles(composeDir);
|
||||||
|
|
||||||
const manifestSize = Object.keys(previousManifest.files).length;
|
|
||||||
console.log(`${logPrefix} Deletion sync: manifest has ${manifestSize} file(s)${manifestSize === 0 ? ' (first sync — nothing will be deleted)' : ''}`);
|
console.log(`${logPrefix} Deletion sync: manifest has ${manifestSize} file(s)${manifestSize === 0 ? ' (first sync — nothing will be deleted)' : ''}`);
|
||||||
|
|
||||||
// First sync / legacy manifest: nothing was recorded, so nothing can be deleted
|
// First sync / legacy manifest: nothing was recorded, so nothing can be deleted
|
||||||
@@ -1236,12 +1258,17 @@ export async function syncGitStack(stackId: number): Promise<SyncResult> {
|
|||||||
console.log(`${logPrefix} No env file path configured`);
|
console.log(`${logPrefix} No env file path configured`);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Deletion sync (#966): manifest-vs-clone deletion plan
|
// Deletion sync (#966): manifest-vs-clone deletion plan.
|
||||||
|
// `updated` is the dir-scoped git-diff result (false when the commit didn't
|
||||||
|
// change OR no compose-dir file changed) - lets the plan skip re-hashing an
|
||||||
|
// unchanged tree.
|
||||||
const deletionData = await computeSyncDeletionPlan({
|
const deletionData = await computeSyncDeletionPlan({
|
||||||
logPrefix,
|
logPrefix,
|
||||||
composeDir,
|
composeDir,
|
||||||
composeFileName,
|
composeFileName,
|
||||||
rawManifest: gitStack.syncedFiles
|
rawManifest: gitStack.syncedFiles,
|
||||||
|
changedInComposeDir: updated,
|
||||||
|
diffBaselineCommit: previousCommit
|
||||||
});
|
});
|
||||||
|
|
||||||
// Update git stack status
|
// Update git stack status
|
||||||
@@ -1875,13 +1902,17 @@ export async function deployGitStackWithProgress(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Deletion sync (#966): manifest-vs-clone deletion plan
|
// Deletion sync (#966): manifest-vs-clone deletion plan. `updated` is the
|
||||||
|
// dir-scoped git-diff result; when nothing changed the plan skips re-hashing
|
||||||
|
// the whole tree.
|
||||||
const logPrefix = `[Stack:${gitStack.stackName}]`;
|
const logPrefix = `[Stack:${gitStack.stackName}]`;
|
||||||
const deletionData = await computeSyncDeletionPlan({
|
const deletionData = await computeSyncDeletionPlan({
|
||||||
logPrefix,
|
logPrefix,
|
||||||
composeDir,
|
composeDir,
|
||||||
composeFileName: progressComposeFileName,
|
composeFileName: progressComposeFileName,
|
||||||
rawManifest: gitStack.syncedFiles
|
rawManifest: gitStack.syncedFiles,
|
||||||
|
changedInComposeDir: updated,
|
||||||
|
diffBaselineCommit: previousCommit
|
||||||
});
|
});
|
||||||
|
|
||||||
// Update git stack status
|
// Update git stack status
|
||||||
|
|||||||
@@ -0,0 +1,39 @@
|
|||||||
|
import { describe, test, expect } from 'bun:test';
|
||||||
|
import { canSkipTreeRehash } from '../src/lib/server/git-deletions';
|
||||||
|
|
||||||
|
const A = 'aaaaaaa1111111111111111111111111111111111';
|
||||||
|
const A7 = 'aaaaaaa'; // same 7-char prefix, DB-stored short form
|
||||||
|
const B = 'bbbbbbb2222222222222222222222222222222222';
|
||||||
|
|
||||||
|
describe('canSkipTreeRehash', () => {
|
||||||
|
test('skips when no change, prior manifest, and baselines match (full vs full)', () => {
|
||||||
|
expect(canSkipTreeRehash(false, 5, A, A)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('skips when baselines match by 7-char prefix (manifest full vs DB short)', () => {
|
||||||
|
expect(canSkipTreeRehash(false, 5, A, A7)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('does NOT skip when the diff baseline drifted from the manifest commit', () => {
|
||||||
|
// bare Sync advanced lastCommit to B while manifest stayed at A -> must re-hash
|
||||||
|
expect(canSkipTreeRehash(false, 5, A, B)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('does NOT skip when files changed', () => {
|
||||||
|
expect(canSkipTreeRehash(true, 5, A, A)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('does NOT skip on the first sync (empty manifest must be built)', () => {
|
||||||
|
expect(canSkipTreeRehash(false, 0, A, A)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('does NOT skip when change state is unknown (new clone / diff failed)', () => {
|
||||||
|
expect(canSkipTreeRehash(undefined, 5, A, A)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('does NOT skip when either commit is null/empty', () => {
|
||||||
|
expect(canSkipTreeRehash(false, 5, null, A)).toBe(false);
|
||||||
|
expect(canSkipTreeRehash(false, 5, A, null)).toBe(false);
|
||||||
|
expect(canSkipTreeRehash(false, 5, A, '')).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in new issue
Block a user