Skip to content

Commit 653b6eb

Browse files
authored
perf: reduce Redis round-trips in pub/sub and batch-job state (#3822)
## What Two independent Redis efficiency improvements, no behavior change: ### 1. Reuse the singleton `ObjectMapper` for pub/sub serialization `RedisWebsocketEventPublisher`, `BatchJobChunkExecutionQueue` and `BatchJobCancellationManager` each constructed a fresh `jacksonObjectMapper()` on every publish — on the websocket and batch hot paths. They now use the injected singleton, matching how `RedisPubSubReceiver` already consumes it. ### 2. Pipeline batch-job state cleanup with Redisson `RBatch` `RedisBatchJobStateStorage` issued one Redis round-trip per key: - **`removeJobState`** deleted the state hash, 7 counters and 2 marker buckets as ~10 sequential calls → now one `RBatch`. - **`clearUnusedStates`** read 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` / `...PerformanceTest` updated for the new constructor arg. New `RedisBatchJobStateStorageTest` (integration, real Redis) covers the parts with no prior coverage: - `removeJobState` pipelines its deletes into a single `RBatch` (spy asserts `createBatch` once, never per-key `getAtomicLong`) - `clearUnusedStates` clears completed jobs across multiple chunks while preserving counters, the started marker and unfinished jobs - `clearUnusedStates` skips a job whose state cannot be read and still cleans the healthy ones `clearUnusedStates` previously had **no** test coverage (it is only driven by a `@Scheduled` cleaner); `removeJobState`'s hash-clearing remains covered end-to-end by `BatchJobsGeneralWithRedisTest`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Improvements** - Improved Redis-backed batch job cleanup and removal while preserving active job information and progress counters. - Enhanced reliability when individual job states cannot be read. - Improved processing of larger sets of completed jobs. - Standardized serialization for Redis-backed batch jobs and WebSocket events for more consistent operation. - **Tests** - Added coverage for cleanup, batched deletion, chunk boundaries, partial read failures, and preservation of unfinished jobs. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 2940a13 commit 653b6eb

8 files changed

Lines changed: 255 additions & 41 deletions

File tree

backend/api/src/main/kotlin/io/tolgee/websocket/RedisWebsocketEventPublisher.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
package io.tolgee.websocket
22

33
import org.springframework.data.redis.core.StringRedisTemplate
4-
import tools.jackson.module.kotlin.jacksonObjectMapper
4+
import tools.jackson.databind.ObjectMapper
55

66
class RedisWebsocketEventPublisher(
77
private val redisTemplate: StringRedisTemplate,
8+
private val objectMapper: ObjectMapper,
89
) : WebsocketEventPublisher {
910
override operator fun invoke(
1011
destination: String,
1112
message: WebsocketEvent,
1213
) {
13-
val messageString = jacksonObjectMapper().writeValueAsString(RedisWebsocketEventWrapper(destination, message))
14+
val messageString = objectMapper.writeValueAsString(RedisWebsocketEventWrapper(destination, message))
1415
redisTemplate.convertAndSend(
1516
"websocket",
1617
messageString,

backend/api/src/main/kotlin/io/tolgee/websocket/WebsocketPublisherConfiguration.kt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import org.springframework.context.annotation.Bean
66
import org.springframework.context.annotation.Configuration
77
import org.springframework.data.redis.core.StringRedisTemplate
88
import org.springframework.messaging.simp.SimpMessagingTemplate
9+
import tools.jackson.databind.ObjectMapper
910

1011
@Configuration
1112
class WebsocketPublisherConfiguration(
@@ -15,7 +16,10 @@ class WebsocketPublisherConfiguration(
1516
@Bean
1617
fun websocketEventPublisher(): WebsocketEventPublisher {
1718
if (websocketProperties.useRedis) {
18-
return RedisWebsocketEventPublisher(applicationContext.getBean(StringRedisTemplate::class.java))
19+
return RedisWebsocketEventPublisher(
20+
applicationContext.getBean(StringRedisTemplate::class.java),
21+
applicationContext.getBean(ObjectMapper::class.java),
22+
)
1923
}
2024
return SimpleWebsocketEventPublisher(applicationContext.getBean(SimpMessagingTemplate::class.java))
2125
}
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
package io.tolgee.batch.state
2+
3+
import io.tolgee.AbstractSpringTest
4+
import io.tolgee.fixtures.RedisRunner
5+
import io.tolgee.model.batch.BatchJobChunkExecutionStatus
6+
import io.tolgee.testing.ContextRecreatingTest
7+
import io.tolgee.testing.assert
8+
import org.junit.jupiter.api.AfterAll
9+
import org.junit.jupiter.api.AfterEach
10+
import org.junit.jupiter.api.Test
11+
import org.mockito.kotlin.any
12+
import org.mockito.kotlin.clearInvocations
13+
import org.mockito.kotlin.never
14+
import org.mockito.kotlin.times
15+
import org.mockito.kotlin.verify
16+
import org.redisson.api.RedissonClient
17+
import org.redisson.client.codec.StringCodec
18+
import org.springframework.beans.factory.annotation.Autowired
19+
import org.springframework.boot.test.context.SpringBootTest
20+
import org.springframework.boot.test.util.TestPropertyValues
21+
import org.springframework.context.ApplicationContextInitializer
22+
import org.springframework.context.ConfigurableApplicationContext
23+
import org.springframework.test.annotation.DirtiesContext
24+
import org.springframework.test.context.ContextConfiguration
25+
import org.springframework.test.context.bean.override.mockito.MockitoSpyBean
26+
27+
@ContextRecreatingTest
28+
@SpringBootTest(
29+
properties = [
30+
"tolgee.cache.use-redis=true",
31+
"tolgee.cache.enabled=true",
32+
"tolgee.websocket.use-redis=true",
33+
],
34+
)
35+
@ContextConfiguration(initializers = [RedisBatchJobStateStorageTest.Companion.Initializer::class])
36+
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS)
37+
class RedisBatchJobStateStorageTest : AbstractSpringTest() {
38+
companion object {
39+
private const val STATE_KEY_PREFIX = "batch_job_state:"
40+
private const val STATE_INITIALIZED_PREFIX = "batch_job_state_initialized:"
41+
private const val STARTED_PREFIX = "batch_job_started:"
42+
43+
val redisRunner = RedisRunner()
44+
45+
@AfterAll
46+
@JvmStatic
47+
fun stopRedis() {
48+
redisRunner.stop()
49+
}
50+
51+
class Initializer : ApplicationContextInitializer<ConfigurableApplicationContext> {
52+
override fun initialize(configurableApplicationContext: ConfigurableApplicationContext) {
53+
redisRunner.run()
54+
TestPropertyValues
55+
.of("spring.data.redis.port=${RedisRunner.port}")
56+
.applyTo(configurableApplicationContext)
57+
}
58+
}
59+
}
60+
61+
@Autowired
62+
lateinit var stateProvider: BatchJobStateProvider
63+
64+
@MockitoSpyBean
65+
@Autowired
66+
lateinit var redissonClient: RedissonClient
67+
68+
private val usedJobIds = mutableSetOf<Long>()
69+
70+
@AfterEach
71+
fun cleanup() {
72+
usedJobIds.forEach { stateProvider.removeJobState(it) }
73+
usedJobIds.clear()
74+
}
75+
76+
@Test
77+
fun `removeJobState pipelines its deletes into a single RBatch`() {
78+
val jobId =
79+
seed(900_020L, BatchJobChunkExecutionStatus.SUCCESS) {
80+
incrementRunningCount(it)
81+
tryMarkJobStarted(it)
82+
}
83+
84+
clearInvocations(redissonClient)
85+
stateProvider.removeJobState(jobId)
86+
87+
// The deletes must be pipelined through one RBatch, not issued as per-key round-trips.
88+
verify(redissonClient, times(1)).createBatch()
89+
verify(redissonClient, never()).getAtomicLong(any<String>())
90+
91+
stateProvider.hasCachedJobState(jobId).assert.isFalse()
92+
}
93+
94+
@Test
95+
fun `clearUnusedStates clears completed jobs across chunks and keeps counters and unfinished jobs`() {
96+
// Exceeds the production CLEANUP_BATCH_SIZE (100) so cleanup runs across more than one chunk.
97+
val completedJobIds = (901_000L until 901_150L).toList()
98+
completedJobIds.forEach {
99+
seed(it, BatchJobChunkExecutionStatus.SUCCESS) {
100+
incrementRunningCount(it)
101+
tryMarkJobStarted(it)
102+
}
103+
}
104+
val runningJobId = seed(900_002L, BatchJobChunkExecutionStatus.RUNNING)
105+
106+
stateProvider.clearUnusedStates()
107+
108+
// Every completed job's hash is cleared — including those in the second chunk.
109+
completedJobIds.forEach { stateProvider.hasCachedJobState(it).assert.isFalse() }
110+
111+
// Counters and the started marker survive until removeJobState finalizes the job
112+
// (dropping the started marker would allow re-execution).
113+
val sample = completedJobIds.first()
114+
isInitializedBucketPresent(sample).assert.isFalse()
115+
stateProvider.getRunningCount(sample).assert.isEqualTo(1)
116+
isStartedBucketPresent(sample).assert.isTrue()
117+
118+
// Unfinished job is left untouched.
119+
stateProvider.hasCachedJobState(runningJobId).assert.isTrue()
120+
isInitializedBucketPresent(runningJobId).assert.isTrue()
121+
}
122+
123+
@Test
124+
fun `clearUnusedStates skips a job whose state cannot be read and still cleans healthy jobs`() {
125+
val healthyJobId = seed(900_040L, BatchJobChunkExecutionStatus.SUCCESS)
126+
val corruptJobId = 900_041L
127+
usedJobIds += corruptJobId
128+
// Forge a value the state codec cannot deserialize, so this job's read future fails.
129+
redissonClient.getMap<String, String>("$STATE_KEY_PREFIX$corruptJobId", StringCodec.INSTANCE)["1"] = "corrupt"
130+
redissonClient.getBucket<Boolean>("$STATE_INITIALIZED_PREFIX$corruptJobId").set(true)
131+
132+
stateProvider.clearUnusedStates()
133+
134+
// The unreadable job is skipped, not deleted...
135+
stateProvider.hasCachedJobState(corruptJobId).assert.isTrue()
136+
// ...while the healthy completed job in the same pass is still cleaned.
137+
stateProvider.hasCachedJobState(healthyJobId).assert.isFalse()
138+
}
139+
140+
private fun seed(
141+
jobId: Long,
142+
status: BatchJobChunkExecutionStatus,
143+
counters: BatchJobStateProvider.(Long) -> Unit = {},
144+
): Long {
145+
usedJobIds += jobId
146+
stateProvider.updateSingleExecution(jobId, 1L, executionState(status))
147+
redissonClient.getBucket<Boolean>("$STATE_INITIALIZED_PREFIX$jobId").set(true)
148+
stateProvider.counters(jobId)
149+
return jobId
150+
}
151+
152+
private fun executionState(status: BatchJobChunkExecutionStatus) =
153+
ExecutionState(
154+
successTargetsCount = 0,
155+
status = status,
156+
chunkNumber = 0,
157+
retry = null,
158+
transactionCommitted = true,
159+
)
160+
161+
private fun isInitializedBucketPresent(jobId: Long) =
162+
redissonClient.getBucket<Boolean>("$STATE_INITIALIZED_PREFIX$jobId").isExists
163+
164+
private fun isStartedBucketPresent(jobId: Long) = redissonClient.getBucket<Boolean>("$STARTED_PREFIX$jobId").isExists
165+
}

backend/data/src/main/kotlin/io/tolgee/batch/BatchJobCancellationManager.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import org.springframework.data.redis.core.StringRedisTemplate
2020
import org.springframework.stereotype.Component
2121
import org.springframework.transaction.PlatformTransactionManager
2222
import org.springframework.transaction.annotation.Transactional
23-
import tools.jackson.module.kotlin.jacksonObjectMapper
23+
import tools.jackson.databind.ObjectMapper
2424

2525
@Component
2626
class BatchJobCancellationManager(
@@ -36,6 +36,7 @@ class BatchJobCancellationManager(
3636
private val batchJobChunkExecutionQueue: BatchJobChunkExecutionQueue,
3737
private val concurrentExecutionLauncher: BatchJobConcurrentLauncher,
3838
private val batchProperties: io.tolgee.configuration.tolgee.BatchProperties,
39+
private val objectMapper: ObjectMapper,
3940
) : Logging {
4041
@Transactional
4142
fun cancel(id: Long) {
@@ -63,7 +64,7 @@ class BatchJobCancellationManager(
6364
if (usingRedisProvider.areWeUsingRedis) {
6465
redisTemplate.convertAndSend(
6566
RedisPubSubReceiverConfiguration.JOB_CANCEL_TOPIC,
66-
jacksonObjectMapper().writeValueAsString(id),
67+
objectMapper.writeValueAsString(id),
6768
)
6869
}
6970
cancelLocalJob(id)

backend/data/src/main/kotlin/io/tolgee/batch/BatchJobChunkExecutionQueue.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import org.springframework.data.redis.core.StringRedisTemplate
2323
import org.springframework.scheduling.annotation.Scheduled
2424
import org.springframework.stereotype.Component
2525
import org.springframework.transaction.annotation.Transactional
26-
import tools.jackson.module.kotlin.jacksonObjectMapper
26+
import tools.jackson.databind.ObjectMapper
2727
import java.util.concurrent.ConcurrentHashMap
2828
import java.util.concurrent.ConcurrentLinkedDeque
2929
import java.util.concurrent.atomic.AtomicInteger
@@ -35,6 +35,7 @@ class BatchJobChunkExecutionQueue(
3535
@Lazy
3636
private val redisTemplate: StringRedisTemplate,
3737
private val metrics: Metrics,
38+
private val objectMapper: ObjectMapper,
3839
) : Logging,
3940
InitializingBean {
4041
companion object {
@@ -222,7 +223,7 @@ class BatchJobChunkExecutionQueue(
222223
val event = JobQueueItemsEvent(batch, QueueEventType.ADD)
223224
redisTemplate.convertAndSend(
224225
RedisPubSubReceiverConfiguration.JOB_QUEUE_TOPIC,
225-
jacksonObjectMapper().writeValueAsString(event),
226+
objectMapper.writeValueAsString(event),
226227
)
227228
}
228229
return

0 commit comments

Comments
 (0)