mirror of
https://github.com/stablyai/orca.git
synced 2026-10-03 00:02:19 +00:00
test(relay): widen the cell-inventory lock census to writes and the whole tree
The census had two structural blind spots. Its detector required `FROM relay_cells`, so a bare `UPDATE relay_cells ... WHERE cell_id = ?` was invisible even though it takes the same row write lock with no SELECT in front of it -- which is precisely the mechanism a per-cell locking conversion has to move. And it read only assignment-store.ts, so the admission selector and the migration-cell registrar took fleet-wide inventory locks entirely outside the policy the census exists to enforce. Match `UPDATE` / `INSERT INTO` / `DELETE FROM relay_cells` as well as `FROM relay_cells`, and readdir the whole relay source tree instead of one file. Sites are now recorded as `file:method`, and the declaration matcher also recognises top-level functions, since the selector and the registrar are modules of free functions rather than store methods. Widening surfaces 15 previously unpoliced sites: three bare writes in assignment-store (reconcileReservationAccounting, removeSupersededSameCellControls, adjustCellReservationAtomically), four more in methods it already listed for their SELECT, six in cell-admission-selector, and two in cell-admission-migration-registration. No locking behaviour changes. The census stays a ratchet: a new unlisted site still fails it.
This commit is contained in:
@@ -1,4 +1,6 @@
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { readdirSync, readFileSync } from 'node:fs'
|
||||
import { join, relative } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { cellInventoryLockOptions, type CellInventoryLockMode } from './assignment-store.js'
|
||||
|
||||
@@ -55,28 +57,44 @@ const CENSUS: CensusEntry[] = [
|
||||
// second selects from the inventory its single caller has already locked.
|
||||
]
|
||||
|
||||
// Every inline `FROM relay_cells ... FOR UPDATE` outside the named lock helpers,
|
||||
// in source order: whole-table locks in reconciliation and sticky placement,
|
||||
// and single-row locks for a cell the method is already scoped to (heartbeat,
|
||||
// fence, drain generation, configuration, or a reservation adjust that runs
|
||||
// under a lock its caller already holds). A new inline lock fails the census
|
||||
// Every statement outside the named lock helpers that takes a relay_cells row
|
||||
// lock, as `file:method` in file-then-source order: whole-table locks in
|
||||
// reconciliation, sticky placement and the admission boundary, and per-cell
|
||||
// locks or writes for a cell the method is already scoped to (heartbeat, fence,
|
||||
// drain generation, configuration, cell registration, or a reservation adjust
|
||||
// that runs under a lock its caller already holds). A new one fails the census
|
||||
// below until it is listed here; per-connection paths that touch more than one
|
||||
// cell go through lockCellRows so the order is fixed.
|
||||
const NAMED_LOCK_HELPERS = ['lockCellInventory', 'lockGeneralCellInventory', 'lockCellRows']
|
||||
|
||||
const INLINE_CELL_LOCK_SITES = [
|
||||
'reconcileCellsWithOptions',
|
||||
'assignStickyOnce',
|
||||
'recordCellHeartbeat',
|
||||
'attestCellFence',
|
||||
'adoptLegacyCellFence',
|
||||
'commitLegacyCellFenceAdoption',
|
||||
'prepareCellFenceAttempt',
|
||||
'attestCellFenceAttempt',
|
||||
'attestCellFenceAttempt',
|
||||
'configureCell',
|
||||
'assertDrainCellGeneration',
|
||||
'adjustCellReservation'
|
||||
const CELL_ROW_LOCK_SITES = [
|
||||
'assignment-store.ts:reconcileCellsWithOptions',
|
||||
'assignment-store.ts:reconcileCellsWithOptions',
|
||||
'assignment-store.ts:assignStickyOnce',
|
||||
'assignment-store.ts:recordCellHeartbeat',
|
||||
'assignment-store.ts:recordCellHeartbeat',
|
||||
'assignment-store.ts:attestCellFence',
|
||||
'assignment-store.ts:adoptLegacyCellFence',
|
||||
'assignment-store.ts:commitLegacyCellFenceAdoption',
|
||||
'assignment-store.ts:prepareCellFenceAttempt',
|
||||
'assignment-store.ts:attestCellFenceAttempt',
|
||||
'assignment-store.ts:attestCellFenceAttempt',
|
||||
'assignment-store.ts:configureCell',
|
||||
'assignment-store.ts:configureCell',
|
||||
'assignment-store.ts:reconcileReservationAccounting',
|
||||
'assignment-store.ts:assertDrainCellGeneration',
|
||||
'assignment-store.ts:removeSupersededSameCellControls',
|
||||
'assignment-store.ts:adjustCellReservation',
|
||||
'assignment-store.ts:adjustCellReservation',
|
||||
'assignment-store.ts:adjustCellReservationAtomically',
|
||||
'cell-admission-migration-registration.ts:add',
|
||||
'cell-admission-migration-registration.ts:insertCell',
|
||||
'cell-admission-selector.ts:setCellAdmissionBeforeBoundary',
|
||||
'cell-admission-selector.ts:setCellAdmissionBeforeBoundary',
|
||||
'cell-admission-selector.ts:apply',
|
||||
'cell-admission-selector.ts:apply',
|
||||
'cell-admission-selector.ts:inspect',
|
||||
'cell-admission-selector.ts:persistIntent'
|
||||
]
|
||||
|
||||
// The background sweeps, and nothing else. A method reachable from one of these
|
||||
@@ -92,9 +110,23 @@ const REQUEST_ENTRY_FILES = [
|
||||
]
|
||||
|
||||
const DECLARATION = /^ {2}(?:private |public )?(?:static )?(?:async )?([A-Za-z_][\w]*)[(<]/
|
||||
// The selector and the registrar are modules of free functions, not store methods.
|
||||
const FUNCTION_DECLARATION = /^(?:export )?(?:async )?function ([A-Za-z_][\w]*)[(<]/
|
||||
|
||||
const SOURCE_DIRECTORY = fileURLToPath(new URL('.', import.meta.url))
|
||||
|
||||
// The lock census is store-scoped on purpose: lockCellInventory is private, so
|
||||
// only this class can reach it, and the call graph below is same-class.
|
||||
function storeSource(): string[] {
|
||||
return readFileSync(new URL('./assignment-store.ts', import.meta.url), 'utf8').split('\n')
|
||||
return readFileSync(join(SOURCE_DIRECTORY, 'assignment-store.ts'), 'utf8').split('\n')
|
||||
}
|
||||
|
||||
function relaySourceFiles(): string[] {
|
||||
return readdirSync(SOURCE_DIRECTORY, { recursive: true, withFileTypes: true })
|
||||
.filter((entry) => entry.isFile() && entry.name.endsWith('.ts'))
|
||||
.filter((entry) => !entry.name.endsWith('.test.ts'))
|
||||
.map((entry) => relative(SOURCE_DIRECTORY, join(entry.parentPath, entry.name)))
|
||||
.sort()
|
||||
}
|
||||
|
||||
function entryPoints(files: string[]): string[] {
|
||||
@@ -158,6 +190,42 @@ function derivedReachability(lines: string[]): (method: string) => Reachability
|
||||
: 'orphan'
|
||||
}
|
||||
|
||||
// Whole statements, not a fixed window: a wide column list or a raw FOR UPDATE
|
||||
// inside query() must not slip past.
|
||||
const TICK = String.fromCharCode(96)
|
||||
const STATEMENT_CALL = '\\.(queryLocked|query)\\(\\s*' + TICK + '([^' + TICK + ']*)' + TICK
|
||||
const CELL_TABLE_READ = /\bFROM\s+relay_cells\b/
|
||||
// A bare write takes the same row lock with no SELECT in front of it, so a
|
||||
// FROM-only detector is blind to exactly the statements a per-cell conversion
|
||||
// has to move.
|
||||
const CELL_ROW_WRITE = /^\s*(?:UPDATE|INSERT INTO|DELETE FROM)\s+relay_cells\b/
|
||||
|
||||
function cellRowLockSites(file: string, source: string): string[] {
|
||||
const lines = source.split('\n')
|
||||
const bounds: { name: string; start: number }[] = []
|
||||
lines.forEach((line, index) => {
|
||||
const declaration = DECLARATION.exec(line) ?? FUNCTION_DECLARATION.exec(line)
|
||||
if (declaration) bounds.push({ name: declaration[1]!, start: index })
|
||||
})
|
||||
const methodAt = (offset: number): string => {
|
||||
const lineIndex = source.slice(0, offset).split('\n').length - 1
|
||||
let name = '<module>'
|
||||
for (const bound of bounds) if (bound.start <= lineIndex) name = bound.name
|
||||
return name
|
||||
}
|
||||
const sites: string[] = []
|
||||
for (const call of source.matchAll(new RegExp(STATEMENT_CALL, 'g'))) {
|
||||
const statement = call[2]!
|
||||
const writes = CELL_ROW_WRITE.test(statement)
|
||||
if (!writes && !CELL_TABLE_READ.test(statement)) continue
|
||||
if (!writes && call[1] !== 'queryLocked' && !/\bFOR\s+UPDATE\b/.test(statement)) continue
|
||||
const method = methodAt(call.index)
|
||||
if (NAMED_LOCK_HELPERS.includes(method)) continue
|
||||
sites.push(`${file}:${method}`)
|
||||
}
|
||||
return sites
|
||||
}
|
||||
|
||||
function readCallSites(): { method: string; mode: CensusMode }[] {
|
||||
const sites: { method: string; mode: CensusMode }[] = []
|
||||
let method = '<module>'
|
||||
@@ -180,37 +248,11 @@ describe('cell inventory lock call-site census', () => {
|
||||
// Why: the census only sees lockCellInventory calls, so a hand-written
|
||||
// `relay_cells ... FOR UPDATE` would escape classification entirely.
|
||||
it('routes every relay_cells row lock through a named lock helper', () => {
|
||||
const lines = storeSource()
|
||||
const rawSites: string[] = []
|
||||
// Whole statements, not a fixed window: a wide column list or a raw
|
||||
// FOR UPDATE inside query() must not slip past.
|
||||
const source = lines.join('\n')
|
||||
const bounds: { name: string; start: number }[] = []
|
||||
lines.forEach((line, index) => {
|
||||
const declaration = DECLARATION.exec(line)
|
||||
if (declaration) bounds.push({ name: declaration[1]!, start: index })
|
||||
})
|
||||
const methodAt = (offset: number): string => {
|
||||
const lineIndex = source.slice(0, offset).split('\n').length - 1
|
||||
let name = '<module>'
|
||||
for (const bound of bounds) if (bound.start <= lineIndex) name = bound.name
|
||||
return name
|
||||
}
|
||||
const tick = String.fromCharCode(96)
|
||||
const statementCall = new RegExp(
|
||||
'\\.(queryLocked|query)\\(\\s*' + tick + '([^' + tick + ']*)' + tick,
|
||||
'g'
|
||||
const sites = relaySourceFiles().flatMap((file) =>
|
||||
cellRowLockSites(file, readFileSync(join(SOURCE_DIRECTORY, file), 'utf8'))
|
||||
)
|
||||
for (const call of source.matchAll(statementCall)) {
|
||||
const statement = call[2]!
|
||||
if (!/\bFROM\s+relay_cells\b/.test(statement)) continue
|
||||
const locks = call[1] === 'queryLocked' || /\bFOR\s+UPDATE\b/.test(statement)
|
||||
if (!locks) continue
|
||||
const method = methodAt(call.index)
|
||||
if (NAMED_LOCK_HELPERS.includes(method)) continue
|
||||
rawSites.push(method)
|
||||
}
|
||||
expect(rawSites).toEqual(INLINE_CELL_LOCK_SITES)
|
||||
|
||||
expect(sites).toEqual(CELL_ROW_LOCK_SITES)
|
||||
})
|
||||
|
||||
it('leaves no call site taking the inventory without naming a mode', () => {
|
||||
|
||||
Reference in New Issue
Block a user