fix(redis-session): clear shutdown interrupted state correctly#1318
fix(redis-session): clear shutdown interrupted state correctly#1318JGoP-L wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Redis-backed session persistence so that per-key state deletion works (specifically enabling GracefulShutdownManager to clear the persisted shutdown_interrupted flag after it is read), preventing repeated “shutdown-interrupted retry” behavior across restarts.
Changes:
- Implemented
RedisSession.delete(SessionKey, String)to delete an individual persisted state key and remove it from the session tracking set. - Added
removeFromSet(...)support to the Redis client adapter abstraction and implemented it for Jedis, Lettuce, and Redisson adapters. - Added regression tests validating single-key deletion behavior and the one-time clearing behavior in
GracefulShutdownManager.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| agentscope-extensions/agentscope-extensions-session-redis/src/main/java/io/agentscope/core/session/redis/RedisSession.java | Implements per-key deletion and updates session key-tracking metadata accordingly. |
| agentscope-extensions/agentscope-extensions-session-redis/src/main/java/io/agentscope/core/session/redis/RedisClientAdapter.java | Extends the adapter contract with removeFromSet (default no-op) to support metadata cleanup. |
| agentscope-extensions/agentscope-extensions-session-redis/src/main/java/io/agentscope/core/session/redis/jedis/JedisClientAdapter.java | Implements set member removal via srem. |
| agentscope-extensions/agentscope-extensions-session-redis/src/main/java/io/agentscope/core/session/redis/lettuce/LettuceClientAdapter.java | Implements set member removal for both standalone and cluster command paths. |
| agentscope-extensions/agentscope-extensions-session-redis/src/main/java/io/agentscope/core/session/redis/redisson/RedissonClientAdapter.java | Implements set member removal using RSet.remove. |
| agentscope-extensions/agentscope-extensions-session-redis/src/test/java/io/agentscope/core/session/redis/JedisSessionTest.java | Adds regression tests for per-key deletion and shutdown flag clearing behavior. |
| @Override | ||
| public void deleteKeys(String... keys) { | ||
| for (String key : keys) { | ||
| values.remove(key); |
There was a problem hiding this comment.
StatefulRedisClientAdapter.deleteKeys(...) currently only removes entries from the values map. Since RedisClientAdapter.deleteKeys is used to delete any Redis key (including the session _keys set), this test adapter can become inconsistent if a test ever deletes set keys via deleteKeys. Consider removing deleted keys from both values and sets (and optionally dropping the set entry when it becomes empty to better mimic Redis behavior).
| values.remove(key); | |
| values.remove(key); | |
| sets.remove(key); |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…to fix-redis-session-key-delete
AgentScope-Java Version
1.0.12-SNAPSHOT
Description
This PR fixes a Redis session persistence bug where per-key session deletion was not implemented for
RedisSession.Background:
GracefulShutdownManager.checkAndClearShutdownInterrupted(...)relies onsession.delete(sessionKey, "shutdown_interrupted")to clear the shutdown interruption flag after it is read.RedisSessionpreviously inherited the default no-op implementation ofSession.delete(SessionKey, String).shutdown_interruptedflag could remain persisted in Redis and be repeatedly detected on later runs.Changes made:
RedisSession.delete(SessionKey, String)to delete a single persisted state key.removeFromSet(...)support to Redis client adapters so deleted state keys are also removed from the session tracking set.GracefulShutdownManagerclearingshutdown_interruptedonly once.How to test:
Closes #1315
Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn -pl agentscope-extensions/agentscope-extensions-session-redis -am test)