fix: adapt queue metrics to CDS v10 scheduling changes#435
Conversation
CDS v10 enables scheduling by default, which changes how outbox messages are written and processed: - Scheduler.scheduleTask now uses UPSERT instead of INSERT, so db.after 'CREATE' never fires for new messages; listen for both 'CREATE' and 'UPSERT' to stay compatible with CDS 9 and 10 - Scheduler also UPSERTs internal flush markers (service:'scheduling'); skip these to avoid counting them as incoming messages - Retries fire UPSERT with attempts>0; guard incomingMessages increment to only count initial inserts (attempts==null) - Update maxAttempts fallback from 20 to 10 (CDS v10 default) Also add db.pool.max:1 to the test bookshop config: @cap-js/sqlite@3 switched from better-sqlite3 to node:sqlite where each :memory: connection is an isolated database, so without a pool cap of 1 the deploy and queue processor can land on different connections and the table created by deploy is invisible to the processor.
SummaryThe following content is AI-generated and provides a summary of the pull request: Fix: Adapt Queue Metrics to CDS v10 Scheduling ChangesBug Fix🐛 Updated queue metrics collection to remain compatible with both CDS v9 and CDS v10, where the scheduler now uses Changes
PR Bot InformationVersion:
|
There was a problem hiding this comment.
The PR looks reasonable overall — the adaptation to CDS v10 scheduling semantics is well-reasoned and the db.pool.max:1 test fix is correct. The one concern worth addressing is that the attempts == null guard on line 206 may silently stop counting initial messages on CDS 9 if the INSERT payload includes an explicit attempts: 0 value; using (createTaskReq.data.attempts ?? 0) === 0 would be more robust across both CDS versions.
PR Bot Information
Version: 1.24.0
- Correlation ID:
bcb5674a-18ef-4aa3-9942-16730edf944e - LLM:
anthropic--claude-4.6-sonnet - Event Trigger:
pull_request.opened - File Content Strategy: Full file content
| cds.db.after(['CREATE', 'UPSERT'], queueEntity, async (_, createTaskReq) => { | ||
| const tenant = cds.context?.tenant | ||
|
|
||
| const queuedServiceName = JSON.parse(createTaskReq.data.msg || '{}').service ?? createTaskReq.data.target |
There was a problem hiding this comment.
Bug: JSON.parse(createTaskReq.data.msg || '{}') will throw a SyntaxError if msg is a non-empty but malformed JSON string — the || '{}' fallback only guards against falsy values, not invalid JSON.
| const queuedServiceName = JSON.parse(createTaskReq.data.msg || '{}').service ?? createTaskReq.data.target | |
| const queuedServiceName = (() => { try { return JSON.parse(createTaskReq.data.msg || '{}') } catch { return {} } })().service ?? createTaskReq.data.target |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| // > a new task is created in the persistent queue | ||
| tenantQueueStastics(statistics, tenant, queuedServiceName).incomingMessages += 1 | ||
| // In CDS >=10 (scheduling:true), retries also fire UPSERT with attempts > 0 — skip those | ||
| if (createTaskReq.data.attempts == null) { |
There was a problem hiding this comment.
Logic Error: The guard createTaskReq.data.attempts == null assumes that a brand-new message on the CDS 9 INSERT path always has attempts as undefined/null in the request data. If CDS 9 includes attempts: 0 explicitly in the INSERT payload, this condition evaluates to false and the initial message is never counted in incomingMessages. Consider guarding with !createTaskReq.data.attempts (falsy check covering both null/undefined and 0) or (createTaskReq.data.attempts ?? 0) === 0.
| if (createTaskReq.data.attempts == null) { | |
| if ((createTaskReq.data.attempts ?? 0) === 0) { |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
CDS v10 enables scheduling by default, which changes how outbox messages are written and processed:
Also add db.pool.max:1 to the test bookshop config: @cap-js/sqlite@3 switched from better-sqlite3 to node:sqlite where each :memory: connection is an isolated database, so without a pool cap of 1 the deploy and queue processor can land on different connections and the table created by deploy is invisible to the processor.