chore: simplify, modernize (Go 1.26), update deps#215
Conversation
Fix high/med bugs across boltjobs and boltkv: - boltkv/driver.go: Get decoded value as string instead of []byte (type mismatch with Set/MGet — silent data corruption) - boltkv/driver.go: RLock in startGCLoop not deferred; extract gcRun to use defer for panic safety - boltjobs/listener.go: delayedJobsListener cursor.Next() on rolled-back tx (dead tx); switch continue→break with txOk flag - boltjobs/listener.go: duplicate slog key "error" in rollback log; rename second to "rollback_error" - boltjobs/driver.go: create() copied InQueue→PushBucket on restart without deleting source entries (duplicates) Modernize (Go 1.26 idioms): - boltjobs: *uint64 active/delayed pointer atomics → atomic.Uint64 value fields in Driver and Options; update attachDB signature - boltjobs/driver.go: listeners.Add(^uint32(0)) decrement trick → Store(0) (counter is always 0 or 1) - boltjobs/listener.go: bytes.Compare → slices.Compare Simplify: - boltjobs/driver.go: remove dead cond *sync.Cond field (allocated, never read)
|
Warning Review limit reached
More reviews will be available in 22 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR modernizes BoltDB-backed jobs and KV drivers while addressing several correctness issues in job recovery, delayed-job rollback handling, KV decoding, and GC lock cleanup.
Changes:
- Updates job counters to value-style atomics and removes unused condition variable setup.
- Fixes KV
Getdecoding to match stored[]bytevalues and extracts GC work into a defer-protected helper. - Adjusts delayed-job rollback flow and restart recovery handling for in-flight jobs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
boltkv/driver.go |
Fixes Get value decoding and refactors GC loop locking into gcRun. |
boltjobs/listener.go |
Updates atomic usage, delayed-job comparison, and rollback logging/control flow. |
boltjobs/item.go |
Switches item counter references to atomic.Uint64. |
boltjobs/driver.go |
Removes unused condition variable, updates atomic counter usage, and changes restart recovery cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…back In create(), deleting the current bbolt cursor key mid-iteration shifts the underlying inode slice so cursor.Next() skips the following entry. Fix by collecting all InQueueBucket entries into a slice first, then moving them to PushBucket in a separate pass. In delayedJobsListener(), d.pq.Insert was called inside the transaction loop before tx.Commit(), so a commit failure left items in the in-memory priority queue even though the bbolt transaction was rolled back. Fix by accumulating decoded items and inserting them only after a successful commit.
Summary
Fix confirmed bugs, apply Go 1.26 idioms, and remove dead code across
boltjobsandboltkv.Applied fixes
R — Bugs fixed (5)
boltkv/driver.goGetstringwhileSet/MGetencode as[]byte— silent data corruption on every Get; fixed to decode[]byteboltkv/driver.gostartGCLoopclearMu.RLock()not deferred — leaked on any panic insidegc.Range; extractedgcRun()helper withdefer clearMu.RUnlock()boltjobs/listener.godelayedJobsListenercontinueafterrollbackadvanced the cursor on a dead (rolled-back) transaction; replaced withbreak+txOkflagboltjobs/listener.gorollback"error"dropped one value; renamed second key to"rollback_error"boltjobs/driver.gocreatecreate()copied InQueue→PushBucket on restart without deleting source entries, causing duplicate processing after restartM — Modern Go 1.26 idioms (3)
boltjobs/driver.go+item.go+listener.go*uint64pointer atomics →atomic.Uint64value fields inDriverandOptions; updatedattachDBsignatureboltjobs/driver.golisteners.Add(^uint32(0))decrement trick →listeners.Store(0)(counter is always 0 or 1)boltjobs/listener.gobytes.Compare→slices.CompareS — Simplifications (1)
boltjobs/driver.gocond *sync.Condfield (allocated in both constructors, never read)Deps
go get -u all && go mod tidyin root andtests/;go work sync— no version changes (already up to date).