HY-love-sleep opened a new pull request, #7157:
URL: https://github.com/apache/shenyu/pull/7157
### Motivation
`RedisConnectionFactory` opens a lettuce connection pool in its constructor
(`afterPropertiesSet()`)
but had no lifecycle end, so every site that rebuilds its redis client after
a configuration change
could only drop the previous one: its pool and its threads stayed alive.
This is the change #7156 asks
for, and the debt the review of #7153 pointed at.
Fixes #7156.
### What is added
- **`RedisConnectionFactory` implements `DisposableBean`**: `destroy()`
delegates to
`LettuceConnectionFactory.destroy()`, which shuts the pool down. A static
`destroyQuietly(ReactiveRedisConnectionFactory)` is added for the callers
that keep only the reactive
template: it ignores a factory that has no lifecycle and logs a failure
instead of throwing, so a
client that cannot be released never breaks the configuration update that
replaces it.
- **The four sites that replace a redis client now release the previous
one**, after the new one is
installed (so a request never sees a destroyed client):
- `AiTokenLimiterPluginHandler`
- `RateLimiterPluginDataHandler`
- `RedisCache.close()`, which closed a connection but not the factory that
owns the pool
- `SensitiveWordPluginDataHandler`, which also destroys the client in
`removePlugin`
### Notes
- The replaced client is destroyed immediately, like
`CachePluginDataHandler` already does with
`lastCache.close()`: a configuration change may therefore fail a request
that is still holding the
old client. An unchanged configuration does not rebuild anything, so the
common path is untouched.
- No behaviour of the plugins changes otherwise: the same client is created,
cached and used.
### Testing
```
./mvnw -pl
shenyu-infra/shenyu-infra-redis,shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-sensitive-word,\
shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-token-limiter,\
shenyu-plugin/shenyu-plugin-fault-tolerance/shenyu-plugin-ratelimiter,\
shenyu-plugin/shenyu-plugin-cache/shenyu-plugin-cache-redis -am test
```
- `RedisConnectionFactoryTest`: a real factory reports `isRunning()` and
stops doing so after
`destroy()`; `destroyQuietly` destroys a lettuce factory, ignores a
factory without a lifecycle and a
`null` one, and swallows a failing `destroy()`.
- The handler tests of the three plugins that cache a client now assert that
changing the
configuration destroys the replaced client while keeping the new one
running, that an unchanged
configuration does not rebuild it, and (for the sensitive word plugin)
that `removePlugin` releases
it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
