perf: reduce Redis round-trips in pub/sub and batch-job state - #3822
perf: reduce Redis round-trips in pub/sub and batch-job state#3822bdshadow wants to merge 2 commits into
Conversation
Constructing a new ObjectMapper on every publish is expensive and was on the websocket and batch-job hot paths. Inject the shared singleton instead, matching how RedisPubSubReceiver already consumes it.
removeJobState now deletes the state hash, the 7 counters and both markers in a single RBatch instead of ~10 sequential round-trips. clearUnusedStates reads every job's state in one pipelined batch (chunked to bound memory and pipeline size) and deletes the completed ones in a second batch, replacing the previous per-job round-trips. When a pipelined decode fails it falls back to isolated per-job reads so one unreadable job cannot block cleanup of the rest.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughRedis batch-state removal and cleanup now use Redisson batching and chunked reads, while Redis publishers use injected Jackson ChangesRedis batch state and event serialization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RedisBatchJobStateStorage
participant RedissonClient
participant Redis
RedisBatchJobStateStorage->>RedissonClient: read cached job IDs in chunks
RedisBatchJobStateStorage->>RedissonClient: pipeline execution-state reads
RedissonClient->>Redis: read job state hashes
RedisBatchJobStateStorage->>RedissonClient: batch completed-state deletions
RedissonClient->>Redis: delete state hashes and initialization markers
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
What
Two independent Redis efficiency improvements, no behavior change:
1. Reuse the singleton
ObjectMapperfor pub/sub serializationRedisWebsocketEventPublisher,BatchJobChunkExecutionQueueandBatchJobCancellationManagereach constructed a freshjacksonObjectMapper()on every publish — on the websocket and batch hot paths. They now use the injected singleton, matching howRedisPubSubReceiveralready consumes it.2. Pipeline batch-job state cleanup with Redisson
RBatchRedisBatchJobStateStorageissued one Redis round-trip per key:removeJobStatedeleted the state hash, 7 counters and 2 marker buckets as ~10 sequential calls → now oneRBatch.clearUnusedStatesread each job's hash and deleted it one at a time → now reads every job's state in one pipelined batch and deletes the completed ones in a second. Work is chunked (100 jobs/pass) so peak pipeline-response size and materialized state stay bounded regardless of job count. When a pipelined decode fails (e.g. a codec/enum change makes a value unreadable), it falls back to isolated per-job reads so one bad job can't abort cleanup of the rest.Tests
BatchJobChunkExecutionQueueTest/...PerformanceTestupdated for the new constructor arg.New
RedisBatchJobStateStorageTest(integration, real Redis) covers the parts with no prior coverage:removeJobStatepipelines its deletes into a singleRBatch(spy assertscreateBatchonce, never per-keygetAtomicLong)clearUnusedStatesclears completed jobs across multiple chunks while preserving counters, the started marker and unfinished jobsclearUnusedStatesskips a job whose state cannot be read and still cleans the healthy onesclearUnusedStatespreviously had no test coverage (it is only driven by a@Scheduledcleaner);removeJobState's hash-clearing remains covered end-to-end byBatchJobsGeneralWithRedisTest.Summary by CodeRabbit