Skip to content

Commit 27c37fb

Browse files
committed
fixes
1 parent 6640a9b commit 27c37fb

4 files changed

Lines changed: 98 additions & 12 deletions

File tree

src/boss.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,10 @@ class Boss extends EventEmitter implements types.EventsMixin {
206206

207207
if (this.#stopping) return
208208

209-
const warnings = rowsCacheStats.filter(i => i.queuedCount > (i.warningQueueSize || WARNINGS.LARGE_QUEUE.size))
209+
// Coerce with Number(): CockroachDB returns these integer columns as strings, so a bare `>`
210+
// would compare lexicographically ("100" > "9" === false) and silently miss the backlog. On
211+
// standard Postgres these are already numbers, so Number() is a no-op.
212+
const warnings = rowsCacheStats.filter(i => Number(i.queuedCount) > (Number(i.warningQueueSize) || WARNINGS.LARGE_QUEUE.size))
210213

211214
for (const warning of warnings) {
212215
await emitAndPersistWarning(this.#warningContext,

src/manager.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,21 +1073,29 @@ class Manager extends EventEmitter implements types.EventsMixin {
10731073
let count = 0
10741074

10751075
for (const job of jobs) {
1076-
const canRetry = job.retry_count < job.retry_limit
1076+
// CockroachDB returns INT8 columns as strings. These rows come straight from a SELECT *, so
1077+
// unlike fetch/getJobById they are never normalized. Coerce the fields used in arithmetic and
1078+
// comparison below — otherwise `retry_count < retry_limit` is a lexicographic string compare
1079+
// ("9" < "10" === false, wrongly failing a retriable job) and `retry_count + 1` concatenates.
1080+
const retryCount = Number(job.retry_count)
1081+
const retryLimit = Number(job.retry_limit)
1082+
const retryDelay = Number(job.retry_delay)
1083+
const retryDelayMax = job.retry_delay_max != null ? Number(job.retry_delay_max) : null
1084+
1085+
const canRetry = retryCount < retryLimit
10771086
let retried = false
10781087

10791088
if (canRetry) {
10801089
// Calculate start_after for retry
10811090
let startAfter = job.start_after
10821091
if (!job.retry_backoff) {
1083-
startAfter = new Date(Date.now() + job.retry_delay * 1000)
1092+
startAfter = new Date(Date.now() + retryDelay * 1000)
10841093
} else {
1085-
const retryCount = job.retry_count + 1
1086-
const exp = Math.min(16, retryCount)
1087-
const delay = job.retry_delay * (Math.pow(2, exp) / 2 + Math.pow(2, exp) / 2 * Math.random())
1094+
const exp = Math.min(16, retryCount + 1)
1095+
const delay = retryDelay * (Math.pow(2, exp) / 2 + Math.pow(2, exp) / 2 * Math.random())
10881096
// Match the canonical failJobs() SQL: LEAST(retry_delay_max, delay) caps the backoff,
10891097
// treating NULL as "no cap" and 0 as a real cap. (`?:` would wrongly treat 0 as no cap.)
1090-
const cappedDelay = job.retry_delay_max != null ? Math.min(job.retry_delay_max, delay) : delay
1098+
const cappedDelay = retryDelayMax != null ? Math.min(retryDelayMax, delay) : delay
10911099
startAfter = new Date(Date.now() + cappedDelay * 1000)
10921100
}
10931101

@@ -1333,7 +1341,17 @@ class Manager extends EventEmitter implements types.EventsMixin {
13331341

13341342
const { rows } = await this.db.executeSql(query.text, query.values)
13351343

1336-
return Object.assign(queue, rows.at(0) ||
1344+
const stats = rows.at(0)
1345+
1346+
// CockroachDB returns integer columns as strings; normalize the stats counts. (The queue fields
1347+
// merged in below come from getQueueCache -> getQueues, which already normalizes them.)
1348+
if (stats && this.config.backend === 'cockroachdb') {
1349+
for (const field of NUMERIC_QUEUE_FIELDS) {
1350+
if (stats[field] !== undefined && stats[field] !== null) stats[field] = Number(stats[field])
1351+
}
1352+
}
1353+
1354+
return Object.assign(queue, stats ||
13371355
{
13381356
deferredCount: 0,
13391357
queuedCount: 0,

test/distributedDatabaseTest.ts

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,53 @@ helper.describePglite('distributed database mode', { timeout: 20000 }, function
198198
expect(retried.state).toBe('retry')
199199
})
200200

201+
it('should retry, not fail, when the backend returns integer columns as strings', async function () {
202+
// Regression: reinsertFailedJobs read the raw SELECT * rows from the distributed fail path. On
203+
// CockroachDB, INT8 columns come back as strings, so `retry_count < retry_limit` was a
204+
// lexicographic compare — "9" < "10" is false — which permanently failed a job that still had
205+
// retries left. The DISTRIBUTED=true Postgres run can't catch this (node-pg returns numbers
206+
// there), so we simulate CockroachDB's string typing with a wrapper over a real connection.
207+
ctx.boss = await helper.start({ ...ctx.bossConfig, __test__distributed: true })
208+
209+
const jobId = await ctx.boss.send(ctx.schema, { test: 'stringints' }, { retryLimit: 10 })
210+
helper.assertTruthy(jobId)
211+
212+
const [job] = await ctx.boss.fetch(ctx.schema)
213+
expect(job.id).toBe(jobId)
214+
215+
const _db = await helper.getDb()
216+
try {
217+
// Put the job one retry short of its limit. Numerically 9 < 10, so it must still retry; only a
218+
// string comparison ("9" < "10" === false) would wrongly fail it.
219+
await _db.executeSql(`UPDATE ${ctx.schema}.job SET retry_count = 9 WHERE id = $1`, [jobId])
220+
221+
// Wrap the connection so SELECT * rows return integer columns as strings, exactly as
222+
// CockroachDB's driver returns INT8. The fail path's select is the only SELECT * it issues.
223+
const integerColumns = ['priority', 'retry_limit', 'retry_count', 'retry_delay', 'retry_delay_max', 'group_tier', 'expire_seconds', 'deletion_seconds', 'pending_dependencies']
224+
const cockroachLike = {
225+
executeSql: async (text: string, values?: unknown[]) => {
226+
const result = await _db.executeSql(text, values)
227+
if (/^\s*SELECT \* FROM/i.test(text)) {
228+
for (const row of result.rows) {
229+
for (const col of integerColumns) {
230+
if (row[col] !== null && row[col] !== undefined) row[col] = String(row[col])
231+
}
232+
}
233+
}
234+
return result
235+
}
236+
}
237+
238+
await ctx.boss.fail(ctx.schema, jobId, null, { db: cockroachLike })
239+
240+
const retried = await ctx.boss.getJobById(ctx.schema, jobId)
241+
helper.assertTruthy(retried)
242+
expect(retried.state).toBe('retry')
243+
} finally {
244+
await _db.close()
245+
}
246+
})
247+
201248
it('should compose completeDistributed inside a caller transaction and roll back with it', async function () {
202249
ctx.boss = await helper.start({ ...ctx.bossConfig, __test__distributed: true })
203250

@@ -273,11 +320,29 @@ helper.describePglite('distributed database mode', { timeout: 20000 }, function
273320
expect(completed.state).toBe('completed')
274321
})
275322

276-
it('should work with noTablePartitioning mode', async function () {
277-
// This test covers the noPartitioning path in plans.ts (lines 244, 260)
323+
it('should return numeric stats counts under the cockroachdb backend', async function () {
324+
// getQueueStats has its own backend === 'cockroachdb' coercion: the stats counts come from a raw
325+
// stats query, not the normalized getQueues path, so they would otherwise be returned as strings
326+
// on CockroachDB. Selecting the cockroachdb backend on Postgres runs that coercion loop and lets
327+
// us assert the public counts come back as numbers.
328+
ctx.boss = await helper.start({ ...ctx.bossConfig, backend: 'cockroachdb' })
329+
330+
const jobId = await ctx.boss.send(ctx.schema, { test: 'stats' })
331+
helper.assertTruthy(jobId)
332+
333+
const stats = await ctx.boss.getQueueStats(ctx.schema)
334+
expect(typeof stats.queuedCount).toBe('number')
335+
expect(stats.totalCount).toBe(1)
336+
})
337+
338+
it('should construct schema with the yugabytedb backend (no partitioning, no advisory locks)', async function () {
339+
// Exercises the noTablePartitioning + noAdvisoryLocks construction path on plain Postgres by
340+
// selecting the yugabytedb backend, whose only flags are those two (both PostgreSQL-compatible,
341+
// they just remove features). The compatibility flags are derived from `backend` and are not
342+
// settable directly — resolveBackend() overwrites them — so the backend is the only way in.
278343
ctx.boss = await helper.start({
279344
...ctx.bossConfig,
280-
noTablePartitioning: true
345+
backend: 'yugabytedb'
281346
})
282347

283348
// Basic send/fetch to verify everything works

test/pgliteAdapterTest.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ function createFakePglite (): PGliteLike & { calls: Array<{ method: 'query' | 'e
66
const calls: Array<{ method: 'query' | 'exec', text: string, params?: unknown[] }> = []
77
return {
88
calls,
9-
async query (text: string, params?: unknown[]) {
9+
async query (text: string, params?: unknown[]): Promise<{ rows: any[] }> {
1010
calls.push({ method: 'query', text, params })
1111
return { rows: [{ id: '1' }] }
1212
},

0 commit comments

Comments
 (0)