Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,13 @@ describe('ScheduledBackupService', () => {
const second = svc.runNowOrJoin();
expect(first).toEqual({ started: true });
expect(second).toEqual({ started: false });
// let the in-flight cycle settle
await new Promise((r) => setTimeout(r, 50));
// Deterministically JOIN the same in-flight cycle rather than sleeping a
// fixed wall-clock 50ms: runSnapshotCycle returns the live `inFlight` promise
// (never starts a second), so awaiting it waits for the fire-and-forget cycle
// to fully finish writing. The old setTimeout(50) let the cycle outlive the
// wait under CPU load, so afterEach's `fs.rm(dir)` raced the still-writing
// cycle and intermittently threw ENOTEMPTY (rmdir on a non-empty dir).
await svc.runSnapshotCycle('manual');
expect((backup.writeSnapshot as jest.Mock).mock.calls.length).toBe(1);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import express, { type Express, type Request, type Response } from 'express';
import type { Server } from 'node:http';

import express, { type Request, type Response } from 'express';
import request from 'supertest';

import { RequestIdMiddleware } from './request-id.middleware';
Expand All @@ -10,19 +12,32 @@ const UUID_V7 =
/^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;

describe('RequestIdMiddleware', () => {
let app: Express;
// One express app + one persistent ephemeral server for the whole suite,
// matching every other supertest spec in the lib (they all listen once in
// beforeAll). The middleware is stateless — each request still mints a fresh
// id — so there is no per-test state to reset. Passing a fresh `express()`
// app to supertest per test makes it stand up AND tear down a brand-new
// ephemeral server for every request; under full-suite CPU saturation that
// rapid listen/connect/close churn intermittently surfaced as a client-side
// "socket hang up", the sole source of this suite's flakiness (TEST-0006).
let server: Server;

beforeEach(() => {
beforeAll((done) => {
const mw = new RequestIdMiddleware();
app = express();
const app = express();
app.use((req: Request, res: Response, next) => mw.use(req, res, next));
app.get('/probe', (req: Request, res: Response) => {
res.json({ openbucket: req.openbucket });
});
server = app.listen(0, done);
});

afterAll((done) => {
server.close(done);
});

it('case 1: mints a UUIDv7 and sets both response headers', async () => {
const res = await request(app).get('/probe');
const res = await request(server).get('/probe');

expect(res.status).toBe(200);
const id = res.body.openbucket.requestId;
Expand All @@ -33,21 +48,21 @@ describe('RequestIdMiddleware', () => {

it('case 2: reuses a syntactically valid upstream X-Request-Id', async () => {
const upstream = '0190d9c1-7f32-7c0c-bea5-1f51d1c0b2c4';
const res = await request(app).get('/probe').set('X-Request-Id', upstream);
const res = await request(server).get('/probe').set('X-Request-Id', upstream);

expect(res.body.openbucket.requestId).toBe(upstream);
expect(res.headers['x-request-id']).toBe(upstream);
});

it('case 3: discards a malformed upstream X-Request-Id and mints fresh', async () => {
const res = await request(app).get('/probe').set('X-Request-Id', 'not-a-uuid');
const res = await request(server).get('/probe').set('X-Request-Id', 'not-a-uuid');

expect(res.body.openbucket.requestId).not.toBe('not-a-uuid');
expect(res.body.openbucket.requestId).toMatch(UUID_V7);
});

it('case 4: initializes the placeholder context (kind=s3, receivedAt=0)', async () => {
const res = await request(app).get('/probe');
const res = await request(server).get('/probe');

expect(res.body.openbucket.kind).toBe('s3');
expect(res.body.openbucket.receivedAt).toBe(0);
Expand Down
33 changes: 18 additions & 15 deletions libs/nestjs/src/lib/s3/concurrency.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,23 @@ describe('concurrency invariants (TEST-0317)', () => {
await fs.rm(dataDir, { recursive: true, force: true });
});

// QUARANTINED (flaky) — these two cases assert concurrency invariants the write
// path does not yet guarantee for writers racing on the SAME target, so they
// fail intermittently / platform-dependently and destabilise CI:
// • same-partNumber UploadPart: both writers rename(2) onto the same `<n>.part`.
// POSIX overwrites atomically (last-wins), but Windows rejects rename-over-
// existing, so the call rejects on a dev box.
// • concurrent first-time same-key PUT: the writer renames the blob BEFORE it
// commits the row; if the losing writer's row commit conflicts, its rollback
// unlinks the shared final blob and tears the winner's result.
// Re-enable after hardening concurrent same-target writes (e.g. per-(bucket,key)
// serialization in ObjectWriterService + rename-over-existing tolerance in
// BlobStore.atomicRename). The deterministic sequential case below stays active.
// Follow-up: harden concurrent same-target writes (see s3/CONCURRENCY.md §4.8).
it.skip('same-partNumber concurrent UploadPart does not throw EEXIST; the part is one whole writer', async () => {
// These two cases assert concurrency invariants for writers racing on the SAME
// target. Both were quarantined (it.skip) while the write path could tear under
// that race; the hardening they waited on has since landed, so they are now
// active and deterministic:
// • same-partNumber UploadPart: each writer stages to a randomUUID-suffixed
// tmp file (no O_EXCL collision) and rename(2)s onto the shared `<n>.part`.
// On POSIX (Linux CI, macOS dev) rename-over-existing is atomic last-wins,
// so the final part is exactly one whole writer's payload — no tear, no
// EEXIST. (Windows rename-over is the only remaining platform caveat; the
// CI runner is Linux.)
// • concurrent first-time same-key PUT: ObjectWriterService now serializes
// writers of the same (bucket,key) through a keyed async mutex
// (`withKeyLock`, F6, commit c87ef90), so the two PUTs run strictly one
// after the other — the loser's rollback can no longer unlink the winner's
// committed blob. Row, blob bytes, and ETag all agree on one winner.
// See s3/CONCURRENCY.md §4.8.
it('same-partNumber concurrent UploadPart does not throw EEXIST; the part is one whole writer', async () => {
const uploadId = 'concurrent-upload';
const a = 'A'.repeat(4096);
const b = 'B'.repeat(8192);
Expand Down Expand Up @@ -136,7 +139,7 @@ describe('concurrency invariants (TEST-0317)', () => {
expect((await fs.readFile(blobs.paths.blobPath('b', 'seq'))).toString()).toBe('second-wins');
});

it.skip('concurrent PUT same key: SQLite serializes the writers; row + blob agree on one winner', async () => {
it('concurrent PUT same key: the per-key write lock serializes the writers; row + blob agree on one winner', async () => {
const x = 'X'.repeat(500);
const y = 'Y'.repeat(700);

Expand Down
Loading