[ISSUE #39] Performance Optimization: Core Module Thread Pool and Lock Management

.github/workflows/pull_request_event.yml

@@ -15,7 +15,9 @@ jobs:
   build:
     name: Test
     runs-on: ubuntu-latest
-    if: ${{ github.event_name == 'pull_request' && (github.event.action == 'opened' || github.event.action == 'synchronize' || github.event.action == 'reopened') }}
+    if: ${{ github.event_name == 'pull_request'
+      && (github.event.action == 'opened' || github.event.action == 'synchronize' ||
+      github.event.action == 'reopened' || github.event.action == 'ready_for_review') }}
     steps:
       - uses: actions/checkout@v4
         with:

CLAUDE.md

@@ -0,0 +1,131 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+## Project Overview
+
+Req-Shield is a Kotlin library that provides request-collapsing functionality for cache-based applications. It prevents the thundering herd problem by ensuring only one request for the same cache key is processed at a time, while subsequent concurrent requests wait for the result.
+
+## Core Architecture
+
+The library is organized into several core modules:
+
+- **core**: Base implementation with synchronous operations
+- **core-reactor**: Reactive implementation using Project Reactor
+- **core-kotlin-coroutine**: Coroutine-based implementation
+- **core-spring**: Spring integration with traditional caching
+- **core-spring-webflux**: Spring WebFlux integration
+- **core-spring-webflux-kotlin-coroutine**: Spring WebFlux + Kotlin Coroutines integration
+- **support**: Shared utilities, models, and constants
+
+### Key Components
+
+1. **ReqShield**: Main orchestrator that manages cache operations and request collapsing
+2. **KeyLock**: Locking mechanism (local or global) to prevent concurrent cache operations
+3. **ReqShieldConfiguration**: Configuration object that defines cache functions, locking behavior, and timeouts
+4. **ReqShieldData**: Wrapper for cached data with metadata (creation time, TTL)
+5. **Spring Aspects**: AOP-based implementations that provide annotation-driven caching
+
+### Design Patterns
+
+- **Template Method**: Core ReqShield logic is template-based with pluggable cache and lock functions
+- **Strategy Pattern**: Different locking strategies (local vs global) and work modes
+- **Aspect-Oriented Programming**: Spring modules use AOP for transparent caching
+- **Factory Pattern**: Configuration objects create appropriate lock implementations
+
+## Development Commands
+
+### Building the Project
+```bash
+./gradlew build                    # Build all modules
+./gradlew :core:build             # Build specific module
+./gradlew clean build             # Clean and build
+```
+
+### Running Tests
+```bash
+./gradlew test                    # Run all tests
+./gradlew :core:test              # Run tests for specific module
+./gradlew test --tests "*ReqShieldTest*"  # Run specific test pattern
+```
+
+### Code Quality
+```bash
+./gradlew ktlintCheck             # Check Kotlin code style
+./gradlew ktlintFormat            # Format Kotlin code
+./gradlew jacocoTestReport        # Generate test coverage report
+```
+
+### Running Examples
+```bash
+./gradlew :req-shield-spring-boot3-example:bootRun
+./gradlew :req-shield-spring-webflux-example:bootRun
+./gradlew :req-shield-spring-webflux-kotlin-coroutine-example:bootRun
+```
+
+## Module Structure
+
+### Core Modules
+Each core module follows the same package structure:
+- `com.linecorp.cse.reqshield.{variant}/` - Main classes (ReqShield, KeyLock implementations)
+- `com.linecorp.cse.reqshield.{variant}/config/` - Configuration classes
+
+### Spring Integration Modules
+Spring modules add:
+- `annotation/` - Cache annotations (@ReqShieldCacheable, @ReqShieldCacheEvict)
+- `aspect/` - AOP implementation for intercepting annotated methods
+- `cache/` - Cache interface implementations
+- `config/` - Auto-configuration for Spring Boot
+
+### Support Module
+Contains shared:
+- `constant/` - Configuration constants and defaults
+- `exception/` - Custom exceptions and error codes
+- `model/` - Data models (ReqShieldData)
+- `utils/` - Utility functions for cache decisions
+
+## Key Configuration Options
+
+### ReqShieldConfiguration Parameters
+- `isLocalLock`: Use local vs distributed locking (default: true)
+- `lockTimeoutMillis`: Lock acquisition timeout (default: 3000ms)
+- `decisionForUpdate`: Percentage of TTL after which to trigger async cache refresh (default: 80)
+- `maxAttemptGetCache`: Max retry attempts when waiting for cache (default: 10)
+- `reqShieldWorkMode`: CREATE_AND_UPDATE_CACHE | ONLY_CREATE_CACHE | ONLY_UPDATE_CACHE
+
+### Work Modes
+- **CREATE_AND_UPDATE_CACHE**: Full functionality (default)
+- **ONLY_CREATE_CACHE**: Never updates existing cache entries
+- **ONLY_UPDATE_CACHE**: Never creates new cache entries
+
+## Testing Guidelines
+
+### Test Infrastructure
+- Uses JUnit 5 platform
+- MockK for Kotlin mocking
+- Testcontainers for integration tests (Redis)
+- Awaitility for asynchronous testing
+- Separate test fixtures in `support` module
+
+### Test Coverage Requirements
+- **Minimum test coverage**: 80% must be maintained across all modules
+- Coverage reports generated via `./gradlew jacocoTestReport`
+- Coverage enforced through Jacoco plugin configuration
+
+### Test Categories
+- **Unit Tests**: Test individual components in isolation
+- **Integration Tests**: Test Spring integration with real Redis containers
+- **Base Test Classes**: Located in `support/src/testFixtures/` for reuse across modules
+
+## Version Compatibility
+
+### Java/Kotlin Compatibility
+- **Core modules**: Java 8+, Kotlin 1.8+
+- **Spring Boot 3 examples**: Java 17+
+- **Spring Boot 2 examples**: Java 8+
+
+### Framework Support
+- Spring Boot 2.7+ (Spring Framework 5.3+)
+- Spring Boot 3.3+
+- Project Reactor 3.4+
+- Kotlin Coroutines 1.7+

core-kotlin-coroutine/src/main/kotlin/com/linecorp/cse/reqshield/kotlin/coroutine/ReqShield.kt

@@ -17,6 +17,7 @@
 package com.linecorp.cse.reqshield.kotlin.coroutine
 
 import com.linecorp.cse.reqshield.kotlin.coroutine.config.ReqShieldConfiguration
+import com.linecorp.cse.reqshield.kotlin.coroutine.config.ReqShieldWorkMode
 import com.linecorp.cse.reqshield.support.constant.ConfigValues.GET_CACHE_INTERVAL_MILLIS
 import com.linecorp.cse.reqshield.support.constant.ConfigValues.MAX_ATTEMPT_SET_CACHE
 import com.linecorp.cse.reqshield.support.constant.ConfigValues.SET_CACHE_RETRY_INTERVAL_MILLIS
@@ -60,7 +61,8 @@ class ReqShield<T>(
         timeToLiveMillis: Long,
     ) {
         val lockType = LockType.UPDATE
-        if (reqShieldConfig.keyLock.tryLock(key, lockType)) {
+
+        fun executeAsyncTask() {
             CoroutineScope(Dispatchers.IO).launch {
                 val reqShieldData =
                     buildReqShieldData(
@@ -75,6 +77,12 @@ class ReqShield<T>(
                 )
             }
         }
+
+        if (reqShieldConfig.reqShieldWorkMode == ReqShieldWorkMode.ONLY_CREATE_CACHE ||
+            reqShieldConfig.keyLock.tryLock(key, lockType)
+        ) {
+            return executeAsyncTask()
+        }
     }
 
     private suspend fun handleLockForCacheCreation(
@@ -83,7 +91,10 @@ class ReqShield<T>(
         timeToLiveMillis: Long,
     ): ReqShieldData<T> {
         val lockType = LockType.CREATE
-        return if (reqShieldConfig.keyLock.tryLock(key, lockType)) {
+
+        return if (reqShieldConfig.reqShieldWorkMode == ReqShieldWorkMode.ONLY_UPDATE_CACHE ||
+            reqShieldConfig.keyLock.tryLock(key, lockType)
+        ) {
             createReqShieldData(key, callable, timeToLiveMillis, lockType)
         } else {
             handleLockFailure(key, callable, timeToLiveMillis)
@@ -177,7 +188,9 @@ class ReqShield<T>(
         } catch (e: Exception) {
             throw ClientException(ErrorCode.SET_CACHE_ERROR, originErrorMessage = e.message)
         } finally {
-            unlockWithRetry(key, lockType)
+            if (shouldAttemptUnlock(lockType)) {
+                unlockWithRetry(key, lockType)
+            }
         }
     }
 
@@ -208,4 +221,8 @@ class ReqShield<T>(
             }
             throw ClientException(ErrorCode.SUPPLIER_ERROR, originErrorMessage = it.message)
         }
+
+    private fun shouldAttemptUnlock(lockType: LockType): Boolean =
+        (lockType == LockType.UPDATE && reqShieldConfig.reqShieldWorkMode != ReqShieldWorkMode.ONLY_CREATE_CACHE) ||
+            (lockType == LockType.CREATE && reqShieldConfig.reqShieldWorkMode != ReqShieldWorkMode.ONLY_UPDATE_CACHE)
 }

core-kotlin-coroutine/src/main/kotlin/com/linecorp/cse/reqshield/kotlin/coroutine/config/ReqShieldConfiguration.kt

@@ -40,6 +40,7 @@ data class ReqShieldConfiguration<T>(
             KeyGlobalLock(globalLockFunction!!, globalUnLockFunction!!, lockTimeoutMillis)
         },
     val maxAttemptGetCache: Int = MAX_ATTEMPT_GET_CACHE,
+    val reqShieldWorkMode: ReqShieldWorkMode = ReqShieldWorkMode.CREATE_AND_UPDATE_CACHE,
 ) {
     init {
         if (!isLocalLock) {
@@ -52,3 +53,9 @@ data class ReqShieldConfiguration<T>(
         }
     }
 }
+
+enum class ReqShieldWorkMode {
+    CREATE_AND_UPDATE_CACHE,
+    ONLY_CREATE_CACHE,
+    ONLY_UPDATE_CACHE,
+}

core-kotlin-coroutine/src/test/kotlin/com/linecorp/cse/reqshield/kotlin/coroutine/ReqShieldTest.kt

@@ -17,6 +17,7 @@
 package com.linecorp.cse.reqshield.kotlin.coroutine
 
 import com.linecorp.cse.reqshield.kotlin.coroutine.config.ReqShieldConfiguration
+import com.linecorp.cse.reqshield.kotlin.coroutine.config.ReqShieldWorkMode
 import com.linecorp.cse.reqshield.support.BaseReqShieldTest
 import com.linecorp.cse.reqshield.support.exception.ClientException
 import com.linecorp.cse.reqshield.support.exception.code.ErrorCode
@@ -51,6 +52,8 @@ import kotlin.test.assertTrue
 @OptIn(ExperimentalCoroutinesApi::class)
 class ReqShieldTest : BaseReqShieldTest {
     private lateinit var reqShield: ReqShield<Product>
+    private lateinit var reqShieldOnlyUpdateCache: ReqShield<Product>
+    private lateinit var reqShieldOnlyCreateCache: ReqShield<Product>
     private lateinit var reqShieldForGlobalLock: ReqShield<Product>
     private lateinit var reqShieldForGlobalLockForError: ReqShield<Product>
     private lateinit var cacheSetter: suspend (String, ReqShieldData<Product>, Long) -> Boolean
@@ -88,6 +91,26 @@ class ReqShieldTest : BaseReqShieldTest {
                 ),
             )
 
+        reqShieldOnlyUpdateCache =
+            ReqShield(
+                ReqShieldConfiguration(
+                    cacheSetter,
+                    cacheGetter,
+                    keyLock = keyLock,
+                    reqShieldWorkMode = ReqShieldWorkMode.ONLY_UPDATE_CACHE,
+                ),
+            )
+
+        reqShieldOnlyCreateCache =
+            ReqShield(
+                ReqShieldConfiguration(
+                    cacheSetter,
+                    cacheGetter,
+                    keyLock = keyLock,
+                    reqShieldWorkMode = ReqShieldWorkMode.ONLY_CREATE_CACHE,
+                ),
+            )
+
         reqShieldForGlobalLock =
             ReqShield(
                 ReqShieldConfiguration(
@@ -128,6 +151,24 @@ class ReqShieldTest : BaseReqShieldTest {
             coVerify { callable() }
         }
 
+    @Test
+    override fun testSetMethodCacheNotExistsAndOnlyUpdateCache() {
+        runBlocking {
+            coEvery { cacheGetter.invoke(key) } returns null
+            coEvery { cacheSetter.invoke(key, any(), any()) } returns true
+
+            val result = reqShieldOnlyUpdateCache.getAndSetReqShieldData(key, callable, timeToLiveMillis)
+            delay(100)
+
+            assertNotNull(result)
+            coVerify { cacheGetter.invoke(key) }
+            coVerify { cacheSetter.invoke(key, result, timeToLiveMillis) }
+            coVerify(inverse = true) { keyLock.tryLock(key, LockType.CREATE) }
+            coVerify(inverse = true) { keyLock.unLock(key, LockType.CREATE) }
+            coVerify { callable() }
+        }
+    }
+
     @Test
     override fun testSetMethodCacheNotExistsAndGlobalLockAcquired() =
         runBlocking {
@@ -394,6 +435,29 @@ class ReqShieldTest : BaseReqShieldTest {
             coVerify { callable() }
         }
 
+    @Test
+    override fun testSetMethodCacheExistsAndTheUpdateTargetOnlyCreateCache() {
+        runBlocking {
+            val timeToLiveMillis: Long = 1000
+            val reqShieldData = ReqShieldData(oldValue, timeToLiveMillis)
+            val newReqShieldData = ReqShieldData(value, timeToLiveMillis)
+
+            coEvery { cacheGetter.invoke(key) } returns reqShieldData
+            coEvery { cacheSetter.invoke(key, any(), any()) } coAnswers { true }
+
+            val result = reqShieldOnlyCreateCache.getAndSetReqShieldData(key, callable, timeToLiveMillis)
+
+            delay(100)
+
+            assertEquals(reqShieldData, result)
+            coVerify { cacheGetter.invoke(key) }
+            coVerify { cacheSetter.invoke(key, newReqShieldData, timeToLiveMillis) }
+            coVerify(inverse = true) { keyLock.tryLock(key, LockType.UPDATE) }
+            coVerify(inverse = true) { keyLock.unLock(key, LockType.UPDATE) }
+            coVerify { callable() }
+        }
+    }
+
     @Test
     override fun testSetMethodCacheExistsAndTheUpdateTargetAndCallableReturnNull() =
         runBlocking {