authorgravatar for 62337003+Bil0000@users.noreply.github.comBilal Bakr <62337003+Bil0000@users.noreply.github.com> 2026-09-15 09:52:43+03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-14 23:52:43-07:00
log5b377e2a047569ff928ccc6b07a900f698332496
tree72519b6949e4d959df2132f3e189fe63e0bb5fcc
parentcc839c42b1adfddd059142bb6d7151719080c4ea
signature Signed by PGP key B5690EEEBB952194

fix(server): bound Git process bursts to keep connections responsive (#11405)

2 files changed, 114 insertions(+), 0 deletions(-)

apps/server/src/vcs/GitVcsDriverCore.test.ts+109
...@@ -8,8 +8,10 @@ import * as Effect from "effect/Effect";...@@ -8,8 +8,10 @@ import * as Effect from "effect/Effect";
8import * as FileSystem from "effect/FileSystem";8import * as FileSystem from "effect/FileSystem";
9import * as Fiber from "effect/Fiber";9import * as Fiber from "effect/Fiber";
10import * as Layer from "effect/Layer";10import * as Layer from "effect/Layer";
11import * as Metric from "effect/Metric";
11import * as Path from "effect/Path";12import * as Path from "effect/Path";
12import * as PlatformError from "effect/PlatformError";13import * as PlatformError from "effect/PlatformError";
14import * as Queue from "effect/Queue";
13import * as Ref from "effect/Ref";15import * as Ref from "effect/Ref";
14import * as Result from "effect/Result";16import * as Result from "effect/Result";
15import * as Scope from "effect/Scope";17import * as Scope from "effect/Scope";
...@@ -20,6 +22,7 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";...@@ -20,6 +22,7 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
2022
21import { GitCommandError, type ReviewDiffFileContentsInput } from "@t3tools/contracts";23import { GitCommandError, type ReviewDiffFileContentsInput } from "@t3tools/contracts";
22import { ServerConfig } from "../config.ts";24import { ServerConfig } from "../config.ts";
25import { gitCommandDuration } from "../observability/Metrics.ts";
23import {26import {
24 makeGitVcsDriverCore,27 makeGitVcsDriverCore,
25 parseGitCheckoutProgressLine,28 parseGitCheckoutProgressLine,
...@@ -136,6 +139,112 @@ const initRepoWithCommit = (...@@ -136,6 +139,112 @@ const initRepoWithCommit = (
136 return { initialBranch };139 return { initialBranch };
137 });140 });
138141
142it.effect("bounds Git bursts across drivers without timing out queued commands", () =>
143 Effect.gen(function* () {
144 const gate = yield* Deferred.make<void>();
145 const starts = yield* Queue.unbounded<number>();
146 let active = 0;
147 let peak = 0;
148 const spawner = ChildProcessSpawner.make(() =>
149 Effect.acquireRelease(
150 Effect.gen(function* () {
151 peak = Math.max(peak, ++active);
152 yield* Queue.offer(starts, active);
153 return ChildProcessSpawner.makeHandle({
154 ...makeSuccessfulHandle("ok"),
155 exitCode: Deferred.await(gate).pipe(Effect.as(ChildProcessSpawner.ExitCode(0))),
156 });
157 }),
158 () => Effect.sync(() => active--),
159 ),
160 );
161 const drivers = yield* Effect.all(
162 Array.from({ length: 16 }, () =>
163 makeGitVcsDriverCore().pipe(
164 Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
165 ),
166 ),
167 );
168 const burst = yield* Effect.forEach(
169 drivers,
170 (driver, index) =>
171 driver.execute({
172 operation: "test.gitBurst",
173 cwd: "/repo",
174 args: ["rev-parse", "HEAD"],
175 ...(index < 4 ? {} : { timeoutMs: index < 8 ? 30_000 : 1_000 }),
176 }),
177 { concurrency: "unbounded" },
178 ).pipe(Effect.forkChild);
179
180 yield* TestClock.adjust("2 seconds");
181 assert.equal(yield* Queue.size(starts), 8);
182 assert.equal(peak, 8);
183 yield* Deferred.succeed(gate, undefined);
184 const results = yield* Fiber.join(burst);
185 assert.equal(results.length, 16);
186 assert.isTrue(results.every((result) => result.stdout === "ok" && result.exitCode === 0));
187 assert.equal(peak, 8);
188 assert.equal(active, 0);
189 const duration = yield* Metric.value(
190 Metric.withAttributes(gitCommandDuration, [["operation", "test.gitBurst"]]),
191 );
192 assert.equal(duration.count, 16);
193 assert.equal(duration.sum, 16_000);
194 }).pipe(Effect.provide(ServerConfigLayer.pipe(Layer.provideMerge(NodeServices.layer)))),
195);
196
197it.effect.each([{ timeoutMs: null }, { timeoutMs: 30_001 }])(
198 "keeps all Git slots available with a pending command whose timeout is $timeoutMs",
199 ({ timeoutMs }) =>
200 Effect.gen(function* () {
201 const slowGate = yield* Deferred.make<void>();
202 const fastGate = yield* Deferred.make<void>();
203 const starts = yield* Queue.unbounded<void>();
204 let active = 0;
205 const spawner = ChildProcessSpawner.make((command) =>
206 Effect.acquireRelease(
207 Effect.gen(function* () {
208 active++;
209 yield* Queue.offer(starts, undefined);
210 const gate =
211 ChildProcess.isStandardCommand(command) && command.args[0] === "push"
212 ? slowGate
213 : fastGate;
214 return ChildProcessSpawner.makeHandle({
215 ...makeSuccessfulHandle("ok"),
216 exitCode: Deferred.await(gate).pipe(Effect.as(ChildProcessSpawner.ExitCode(0))),
217 });
218 }),
219 () => Effect.sync(() => active--),
220 ),
221 );
222 const driver = yield* makeGitVcsDriverCore().pipe(
223 Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
224 );
225 const slow = yield* driver
226 .execute({ operation: "test.slowGit", cwd: "/repo", args: ["push"], timeoutMs })
227 .pipe(Effect.forkChild);
228 yield* Queue.take(starts);
229 const burst = yield* Effect.all(
230 Array.from({ length: 8 }, () =>
231 driver.execute({ operation: "test.fastGit", cwd: "/repo", args: ["status"] }),
232 ),
233 { concurrency: "unbounded" },
234 ).pipe(Effect.forkChild);
235
236 yield* TestClock.adjust("0 seconds");
237 assert.equal(yield* Queue.size(starts), 8);
238 assert.equal(active, 9);
239 yield* Deferred.succeed(fastGate, undefined);
240 assert.equal((yield* Fiber.join(burst)).length, 8);
241 assert.equal(active, 1);
242 yield* Deferred.succeed(slowGate, undefined);
243 assert.equal((yield* Fiber.join(slow)).stdout, "ok");
244 assert.equal(active, 0);
245 }).pipe(Effect.provide(ServerConfigLayer.pipe(Layer.provideMerge(NodeServices.layer)))),
246);
247
139for (const location of ["root", "nested", "worktree"] as const) {248for (const location of ["root", "nested", "worktree"] as const) {
140 it.effect(249 it.effect(
141 `skips clean filters while the ${location} index is locked and resumes after unlock`,250 `skips clean filters while the ${location} index is locked and resumes after unlock`,
apps/server/src/vcs/GitVcsDriverCore.ts+5
...@@ -39,6 +39,7 @@ import {...@@ -39,6 +39,7 @@ import {
39import { ServerConfig } from "../config.ts";39import { ServerConfig } from "../config.ts";
4040
41const DEFAULT_TIMEOUT_MS = 30_000;41const DEFAULT_TIMEOUT_MS = 30_000;
42const gitProcesses = Semaphore.makeUnsafe(8);
42// `git worktree add` checks out the full tree, so on large repositories it can43// `git worktree add` checks out the full tree, so on large repositories it can
43// take well beyond the default 30s (e.g. a 375k-file repo takes ~40s on an idle44// take well beyond the default 30s (e.g. a 375k-file repo takes ~40s on an idle
44// machine). Give it generous headroom while still bounding a genuinely hung git.45// machine). Give it generous headroom while still bounding a genuinely hung git.
...@@ -903,6 +904,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -903,6 +904,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
903 operation: input.operation,904 operation: input.operation,
904 },905 },
905 }),906 }),
907 (execution) =>
908 input.timeoutMs === null || (input.timeoutMs ?? DEFAULT_TIMEOUT_MS) > DEFAULT_TIMEOUT_MS
909 ? execution
910 : gitProcesses.withPermits(1)(execution),
906 Effect.withSpan(input.operation, {911 Effect.withSpan(input.operation, {
907 kind: "client",912 kind: "client",
908 attributes: {913 attributes: {