mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-26 18:35:05 +00:00
fix(state-store): preserve permission errors after lock retries
This commit is contained in:
1 parent
94cd7db7c3
commit
3768c03653
4 files changed
+51
-23
No files matched your search
@@ -97,8 +97,12 @@ remove the database itself. In-memory stores do not create lock files.
|
||||
|
||||
The control-pane HTTP server runs board claims and moves in one-shot workers,
|
||||
so waiting for another writer does not stall health checks or snapshots. A
|
||||
timed-out mutation returns HTTP 503 with `code: STATE_STORE_BUSY` and
|
||||
`Retry-After: 1`; invalid mutations continue to return HTTP 400. Each worker
|
||||
mutation that times out while lock creation reports `EEXIST` returns HTTP 503
|
||||
with `code: STATE_STORE_BUSY` and `Retry-After: 1`. On Windows, lock creation
|
||||
also retries `EPERM` within the same deadline because deletion may still be
|
||||
pending. If the final attempt still reports `EPERM`, the original permission
|
||||
error is preserved rather than replaced with busy or leftover-lock recovery
|
||||
advice. Invalid mutations continue to return HTTP 400. Each worker
|
||||
closes its store before reporting the result and exits naturally, including
|
||||
when the requesting browser disconnects, to avoid interrupting a write.
|
||||
|
||||
|
||||
@@ -75,10 +75,13 @@ function acquireLock(dbPath, timeoutMs) {
|
||||
} catch (error) {
|
||||
// Windows may report EPERM while the previous owner's unlinked file is
|
||||
// pending deletion. Retry exclusive creation within the same deadline.
|
||||
const pendingDelete = process.platform === 'win32' && hasCode(error, 'EPERM');
|
||||
if (!hasCode(error, 'EEXIST') && !pendingDelete) throw error;
|
||||
const retryablePermissionError = process.platform === 'win32' && hasCode(error, 'EPERM');
|
||||
if (!hasCode(error, 'EEXIST') && !retryablePermissionError) throw error;
|
||||
const remaining = deadline - performance.now();
|
||||
if (remaining <= 0) {
|
||||
// EPERM can also mean genuine access denial; preserve its diagnostics
|
||||
// rather than advising the caller to remove a possibly unrelated lock.
|
||||
if (retryablePermissionError) throw error;
|
||||
const busy = new Error(`State store is busy: ${dbPath}. Retry after the other ECC operation finishes. `
|
||||
+ `If an operation terminated unexpectedly, stop all ECC processes using this database, `
|
||||
+ `then inspect and remove the leftover lock: ${lockPath}`);
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
const workerThreads = require('worker_threads');
|
||||
const OriginalWorker = workerThreads.Worker;
|
||||
const metrics = { active: 0, peak: 0, started: 0, exited: 0, received: 0, disconnected: 0 };
|
||||
let metrics = { active: 0, peak: 0, started: 0, exited: 0, received: 0, disconnected: 0 };
|
||||
function send(message) {
|
||||
if (process.connected) process.send(message, error => { if (error) process.exitCode = 1; });
|
||||
}
|
||||
@@ -10,11 +10,17 @@ function report() { send({ type: 'workers', ...metrics }); }
|
||||
workerThreads.Worker = class ObservedWorker extends OriginalWorker {
|
||||
constructor(...args) {
|
||||
super(...args);
|
||||
metrics.active += 1;
|
||||
metrics.started += 1;
|
||||
metrics.peak = Math.max(metrics.peak, metrics.active);
|
||||
metrics = {
|
||||
...metrics,
|
||||
active: metrics.active + 1,
|
||||
started: metrics.started + 1,
|
||||
peak: Math.max(metrics.peak, metrics.active + 1)
|
||||
};
|
||||
report();
|
||||
this.once('exit', () => { metrics.active -= 1; metrics.exited += 1; report(); });
|
||||
this.once('exit', () => {
|
||||
metrics = { ...metrics, active: metrics.active - 1, exited: metrics.exited + 1 };
|
||||
report();
|
||||
});
|
||||
}
|
||||
};
|
||||
const { createControlPaneServer } = require('../../../scripts/lib/control-pane/server');
|
||||
@@ -26,11 +32,14 @@ async function main() {
|
||||
});
|
||||
app.server.prependListener('request', (req, res) => {
|
||||
if (req.method === 'POST') {
|
||||
metrics.received += 1;
|
||||
metrics = { ...metrics, received: metrics.received + 1 };
|
||||
report();
|
||||
send({ type: 'mutation-received' });
|
||||
res.once('close', () => {
|
||||
if (!res.writableFinished) { metrics.disconnected += 1; report(); }
|
||||
if (!res.writableFinished) {
|
||||
metrics = { ...metrics, disconnected: metrics.disconnected + 1 };
|
||||
report();
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -27,7 +27,7 @@ function thrownBy(fn) {
|
||||
catch (error) { return { failed: true, error }; }
|
||||
}
|
||||
|
||||
function lockFixture(overrides = {}, platform = 'linux') {
|
||||
function lockFixture(overrides = {}, platform = 'linux', performance = require('perf_hooks').performance) {
|
||||
const calls = [];
|
||||
const identity = { dev: 1n, ino: 2n, isFile: () => true, isSymbolicLink: () => false };
|
||||
let present = true;
|
||||
@@ -46,7 +46,7 @@ function lockFixture(overrides = {}, platform = 'linux') {
|
||||
const original = facade[name];
|
||||
facade[name] = (...args) => fn(state, original, ...args);
|
||||
}
|
||||
const lock = loadPrivate(LOCK, { fs: facade, os: { hostname: () => 'fixture' } }, undefined,
|
||||
const lock = loadPrivate(LOCK, { fs: facade, os: { hostname: () => 'fixture' }, perf_hooks: { performance } }, undefined,
|
||||
{ platform, pid: 7 });
|
||||
return { ...state, calls, run: (callback, timeoutMs = 0) => lock.withStateStoreLock('synthetic.db', callback, { timeoutMs }) };
|
||||
}
|
||||
@@ -69,16 +69,28 @@ async function runSynthetic(test) {
|
||||
assert.strictEqual(callbacks, 1);
|
||||
assert.deepStrictEqual(fixture.calls, ['open', 'open', 'fstat', 'write', 'lstat', 'unlink', 'close']);
|
||||
});
|
||||
await test('sustained Windows open EPERM times out as busy without touching the lock', () => {
|
||||
const denied = Object.assign(new Error('pending deletion'), { code: 'EPERM' });
|
||||
const fixture = lockFixture({ openSync(state) { state.calls.push('open'); throw denied; } }, 'win32');
|
||||
const result = thrownBy(() => fixture.run(() => assert.fail('callback must not run'), 20));
|
||||
assert.ok(result.failed);
|
||||
assert.strictEqual(result.error.code, 'STATE_STORE_BUSY');
|
||||
assert.match(result.error.message, /synthetic\.db\.ecc-state\.lock/);
|
||||
assert.ok(fixture.calls.length > 0);
|
||||
assert.ok(fixture.calls.every(call => call === 'open'));
|
||||
});
|
||||
for (const codes of [['EPERM', 'EPERM'], ['EEXIST', 'EPERM'], ['EPERM', 'EEXIST']]) {
|
||||
await test(`Windows lock timeout preserves the final error classification (${codes.join(' -> ')})`, () => {
|
||||
const errors = codes.map(code => Object.assign(new Error(`open failed: ${code}`), { code }));
|
||||
let attempts = 0;
|
||||
const clock = { now: () => attempts * 10 };
|
||||
const fixture = lockFixture({ openSync(state) {
|
||||
state.calls.push('open');
|
||||
throw errors[attempts++];
|
||||
} }, 'win32', clock);
|
||||
const result = thrownBy(() => fixture.run(() => assert.fail('callback must not run'), 20));
|
||||
assert.ok(result.failed);
|
||||
if (codes[1] === 'EPERM') {
|
||||
assert.strictEqual(result.error, errors[1]);
|
||||
assert.doesNotMatch(result.error.message, /remove|leftover lock/i);
|
||||
} else {
|
||||
assert.strictEqual(result.error.code, 'STATE_STORE_BUSY');
|
||||
assert.match(result.error.message, /synthetic\.db\.ecc-state\.lock/);
|
||||
}
|
||||
assert.strictEqual(attempts, 2);
|
||||
assert.deepStrictEqual(fixture.calls, ['open', 'open']);
|
||||
});
|
||||
}
|
||||
for (const [platform, code] of [['linux', 'EPERM'], ['darwin', 'EPERM'], ['win32', 'EACCES'], ['win32', 'EIO']]) {
|
||||
await test(`unrelated lock-open errors propagate immediately (${platform}, ${code})`, () => {
|
||||
const denied = Object.assign(new Error('open denied'), { code });
|
||||
|
||||
Reference in new issue
Block a user