Skip to content

Commit 8d71aba

Browse files
committed
chore: cut the comment density in the async pool changeset
1 parent e96d6c8 commit 8d71aba

12 files changed

Lines changed: 2 additions & 68 deletions

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ class ActivityWebsocketListener(
3232
private val relationDescriptionExtractor: RelationDescriptionExtractor,
3333
private val currentDateProvider: CurrentDateProvider,
3434
) {
35-
// Not the default pool: this one is single-threaded, and the webapp applies these events as
36-
// ordered deltas.
35+
// Ordering-sensitive — keep the qualifier; see websocketAsyncExecutor for why.
3736
@Async("websocketAsyncExecutor")
3837
@EventListener
3938
fun onActivity(event: OnProjectActivityStoredEvent) {

backend/app/src/main/kotlin/io/tolgee/configuration/AsyncExecutorFactory.kt

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,7 @@ class AsyncExecutorFactory(
1515
private val tolgeeProperties: TolgeeProperties,
1616
private val dataSourceProvider: ObjectProvider<DataSource>,
1717
) {
18-
/**
19-
* Null when the DataSource is not a HikariDataSource, which is the only thing that can report a
20-
* pool size. Read from the live bean rather than from `spring.datasource.*`, because
21-
* PostgresAutoStartConfiguration binds those properties straight onto the HikariDataSource and so
22-
* which key wins depends on whether autostart is enabled.
23-
*/
18+
/** Read off the live bean, not `spring.datasource.*` — see PostgresAutoStartConfiguration. */
2419
val connectionPoolSize: Int? by lazy {
2520
(dataSourceProvider.ifAvailable as? HikariDataSource)?.maximumPoolSize
2621
}
@@ -78,7 +73,6 @@ class AsyncExecutorFactory(
7873
const val BACKGROUND_POOL_DIVISOR = 6
7974
const val FALLBACK_CONNECTION_POOL_SIZE = 10
8075

81-
/** Matches the pre-existing @Async behaviour: background work is queued, never rejected. */
8276
const val UNBOUNDED_QUEUE = Int.MAX_VALUE
8377

8478
const val STREAMING_THREAD_NAME_PREFIX = "tolgee-stream-"

backend/app/src/main/kotlin/io/tolgee/configuration/AsyncMethodConfiguration.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ class AsyncMethodConfiguration(
1919
) : AsyncConfigurer {
2020
override fun getAsyncExecutor(): Executor = backgroundAsyncExecutor()
2121

22-
// @Primary so the two special-purpose pools below cannot win a by-type TaskExecutor injection.
2322
@Primary
2423
@Bean(BACKGROUND_EXECUTOR_BEAN_NAME)
2524
fun backgroundAsyncExecutor(): ThreadPoolTaskExecutor {

backend/app/src/test/kotlin/io/tolgee/ExceptionHandlersAsyncCapacityTest.kt

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@ class ExceptionHandlersAsyncCapacityTest {
2929
.isEqualTo(Message.SERVER_BUSY.code)
3030
}
3131

32-
/**
33-
* The streaming return-value handler stages these before the task is submitted; a caching proxy
34-
* would otherwise key the 503 against that ETag.
35-
*/
3632
@Test
3733
fun `drops the streaming headers already staged on the response`() {
3834
val response = MockHttpServletResponse()
@@ -46,7 +42,6 @@ class ExceptionHandlersAsyncCapacityTest {
4642
response.getHeader(HttpHeaders.ETAG).assert.isNull()
4743
}
4844

49-
/** Without these the browser reports a CORS failure instead of surfacing the 503. */
5045
@Test
5146
fun `keeps the CORS and version headers written by earlier filters`() {
5247
val response = MockHttpServletResponse()
@@ -75,7 +70,6 @@ class ExceptionHandlersAsyncCapacityTest {
7570
response.getHeader(HttpHeaders.CONTENT_DISPOSITION).assert.isNotNull
7671
}
7772

78-
/** Any other executor's rejection is not ours to relabel as streaming saturation. */
7973
@Test
8074
fun `sends a rejection from another executor down the generic path`() {
8175
val unrelated = TaskRejectedException("some other executor is shutting down")
@@ -102,7 +96,6 @@ class ExceptionHandlersAsyncCapacityTest {
10296
response.getHeader(HttpHeaders.CONTENT_DISPOSITION).assert.isNull()
10397
}
10498

105-
/** A stream that timed out mid-write cannot be answered, and is worth reporting rather than hiding. */
10699
@Test
107100
fun `reports a stream that timed out after it started writing`() {
108101
val response = MockHttpServletResponse()
@@ -116,7 +109,6 @@ class ExceptionHandlersAsyncCapacityTest {
116109
private fun handle(response: MockHttpServletResponse) =
117110
exceptionHandlers.handleAsyncCapacityExceeded(wrappedRejection(), response)
118111

119-
/** Spring wraps whatever the rejection policy throws, so the handler only ever sees the wrapper. */
120112
private fun wrappedRejection(): RejectedExecutionException =
121113
TaskRejectedException("pool full", StreamingCapacityExceededException("queued=1, active=1"))
122114
}

backend/app/src/test/kotlin/io/tolgee/StreamingBodyDatabasePoolHealthTest.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@ class StreamingBodyDatabasePoolHealthTest : ProjectAuthControllerTest("/v2/proje
6161
val hikariDataSource = dataSource as HikariDataSource
6262
val pool = hikariDataSource.hikariPoolMXBean
6363

64-
// Every streaming response borrows a connection for its whole duration, so a stream that
65-
// fails to hand it back leaves it active forever.
6664
waitForNotThrowing(pollTime = 50, timeout = 5000) {
6765
pool.activeConnections.assert.isEqualTo(0)
6866
}

backend/app/src/test/kotlin/io/tolgee/configuration/AsyncCapacityReporterTest.kt

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import org.slf4j.LoggerFactory
1313
class AsyncCapacityReporterTest {
1414
@Test
1515
fun `stays quiet when a quarter of the connection pool is left for ordinary requests`() {
16-
// 33 streaming + 16 background + 20 batch = 69 of 100, leaving 31.
1716
val events = report(connectionPoolSize = 100, batchConcurrency = 20)
1817

1918
events.warnings.assert.isEmpty()
@@ -22,16 +21,11 @@ class AsyncCapacityReporterTest {
2221

2322
@Test
2423
fun `warns when the pools plus batch jobs crowd out ordinary requests`() {
25-
// 33 streaming + 16 background + 40 batch = 89 of 100.
2624
val events = report(connectionPoolSize = 100, batchConcurrency = 40)
2725

2826
events.warnings.assert.anyMatch { it.contains("may starve the database connection pool") }
2927
}
3028

31-
/**
32-
* Which key actually sets the pool size flips with postgres-autostart, so naming the wrong one
33-
* would send an operator to a property their deployment silently ignores.
34-
*/
3529
@Test
3630
fun `names the connection pool property that the active binding mode actually reads`() {
3731
report(connectionPoolSize = 100, batchConcurrency = 40, postgresAutostart = true)
@@ -45,8 +39,6 @@ class AsyncCapacityReporterTest {
4539

4640
@Test
4741
fun `sits exactly on the boundary without warning, and warns one job past it`() {
48-
// pool 60 -> 20 streaming + 10 background, and the reserve is 60/4 = 15, so 15 batch jobs is
49-
// the last configuration that still leaves a quarter free.
5042
report(connectionPoolSize = 60, batchConcurrency = 15).warnings.assert.isEmpty()
5143
report(connectionPoolSize = 60, batchConcurrency = 16).warnings.assert.isNotEmpty()
5244
}

backend/app/src/test/kotlin/io/tolgee/configuration/AsyncDispatchTargetTest.kt

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,6 @@ import org.springframework.scheduling.annotation.Async
1111
import java.util.concurrent.CompletableFuture
1212
import java.util.concurrent.TimeUnit
1313

14-
/**
15-
* The MVC side has a wiring test; without this the @Async side could silently fall back to Spring's
16-
* own executor, or the websocket qualifier could resolve to the general pool, and every
17-
* configuration assertion would still pass.
18-
*/
1914
@SpringBootTest
2015
@Import(AsyncDispatchTargetTest.AsyncProbeConfiguration::class)
2116
class AsyncDispatchTargetTest {

backend/app/src/test/kotlin/io/tolgee/configuration/AsyncExecutorConfigurationTest.kt

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,6 @@ class AsyncExecutorConfigurationTest {
5151
@Autowired
5252
private lateinit var asyncMethodConfiguration: AsyncMethodConfiguration
5353

54-
/**
55-
* Hard-coded rather than recomputed from the divisors, so a change to either divisor has to be
56-
* made here too instead of silently agreeing with itself.
57-
*/
5854
@Test
5955
fun `derives both pools from the database connection pool`() {
6056
(dataSource as HikariDataSource)
@@ -68,10 +64,6 @@ class AsyncExecutorConfigurationTest {
6864
asyncExecutorFactory.backgroundMaxThreads.assert.isEqualTo(16)
6965
}
7066

71-
/**
72-
* Guards against silently falling back to ThreadPoolTaskExecutor's defaults, which pin a pool to a
73-
* single thread forever because the unbounded queue never lets it grow past core size.
74-
*/
7567
@Test
7668
fun `no pool is left at the single-threaded default`() {
7769
listOf(streamingAsyncExecutor, asyncMethodConfiguration.backgroundAsyncExecutor()).forEach { executor ->
@@ -131,10 +123,6 @@ class AsyncExecutorConfigurationTest {
131123
.isEqualTo(AsyncMethodConfiguration.WEBSOCKET_EXECUTOR_BEAN_NAME)
132124
}
133125

134-
/**
135-
* Without this, dropping the configureAsyncSupport override would silently fall back to Spring's
136-
* own executor and every other test here would still pass.
137-
*/
138126
@Test
139127
fun `mvc async dispatch runs on the streaming executor`() {
140128
val installed = ReflectionTestUtils.getField(requestMappingHandlerAdapter, "taskExecutor")
@@ -144,10 +132,6 @@ class AsyncExecutorConfigurationTest {
144132
delegate.assert.isSameAs(streamingAsyncExecutor)
145133
}
146134

147-
/**
148-
* StreamingBodyDatabasePoolHealthTest was de-flaked on this setting, but it would go back to
149-
* skipping rather than failing if the setting were lost. This fails outright.
150-
*/
151135
@Test
152136
fun `security headers are written before the chain, not while it unwinds`() {
153137
val headerWriterFilter =
@@ -167,7 +151,6 @@ class AsyncExecutorConfigurationTest {
167151
.isInstanceOf(CompositeTaskDecorator::class.java)
168152
}
169153

170-
/** The two divisors and the reporter's reserve are separate constants; defaults must agree. */
171154
@Test
172155
fun `the shipped defaults never trip the capacity warning`() {
173156
val poolSize = asyncExecutorFactory.connectionPoolSize!!
@@ -209,7 +192,6 @@ class AsyncExecutorConfigurationTest {
209192
).apply { initialize() }
210193
val release = CyclicBarrier(2)
211194
try {
212-
// Occupies the single thread, then fills the single queue slot.
213195
executor.submit { release.await(10, TimeUnit.SECONDS) }
214196
executor.submit { }
215197

backend/app/src/test/kotlin/io/tolgee/configuration/AsyncPoolSizeDerivationTest.kt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,6 @@ import io.tolgee.testing.assert
66
import org.junit.jupiter.api.Test
77
import javax.sql.DataSource
88

9-
/**
10-
* Plain unit test so the floor and the "not a Hikari pool" fallback are reachable — a @SpringBootTest
11-
* is stuck with whatever connection pool the test context happens to have.
12-
*/
139
class AsyncPoolSizeDerivationTest {
1410
@Test
1511
fun `derives each pool by its divisor`() {

backend/app/src/test/kotlin/io/tolgee/configuration/AsyncPropertyBindingTest.kt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,6 @@ import org.springframework.beans.factory.annotation.Qualifier
77
import org.springframework.boot.test.context.SpringBootTest
88
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor
99

10-
/**
11-
* The deployment values.yaml pins tolgee.async.* explicitly, so those keys have to survive relaxed
12-
* binding all the way into the executors — nothing else in the suite would notice a rename.
13-
*/
1410
@SpringBootTest(
1511
properties = [
1612
"tolgee.async.streaming.max-threads = 7",

0 commit comments

Comments
 (0)