Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incUsage is not called when acquireConnection -> createConnection #4441

Closed
LYFKiririn opened this issue Jul 25, 2022 · 3 comments
Closed

incUsage is not called when acquireConnection -> createConnection #4441

LYFKiririn opened this issue Jul 25, 2022 · 3 comments
Labels
Milestone

Comments

@LYFKiririn
Copy link

incUsage is not called when acquireConnection -> createConnection
but decUsage is still called when releasrConnection, which lead usage to become -1
after this, PingConnectionHandler will behave incorrectly:

  1. send ping when in usage (poll: -1 to 0)
  2. not send ping when idle (release: 0 to -1)

Expected behavior
incUsage is called when acquireConnection

Actual behavior
incUsage is called when acquire exist free connection
incUsage is not called when acquire create new connection

Steps to reproduce or test case
freeConnections size < acquireConnection size
for example (pseudo-code):

  1. setConnectionMinimumIdleSize(1)
  2. setConnectionPoolSize(8)
  3. Redisson.create(config)
  4. Executor pool = Executors.newFixedThreadPool(8)
  5. for (int i = 0; i < 8; i++) pool.execute(() -> redisson.getBucket("test").getAsync())

Redis version
6.2.6

Redisson version
3.17.4

Redisson configuration
connectionMinimumIdleSize: 1
connectionPoolSize: 8
pingConnectionInterval: 5000
idleConnectionTimeout: 10000

【acquire exist free connection】【release correctly:0 to 1 to 0】
image
image

【acquire create new connection】【release incorrectly:0 to -1】
image
image

【PingConnectionHandler behavior】
image

@mrniko mrniko added this to the 3.17.6 milestone Aug 5, 2022
@mrniko mrniko added the bug label Aug 5, 2022
mrniko pushed a commit that referenced this issue Aug 5, 2022
@mrniko
Copy link
Member

mrniko commented Aug 5, 2022

Fixed. Thanks for report

@mrniko mrniko closed this as completed Aug 5, 2022
@LYFKiririn
Copy link
Author

LYFKiririn commented Aug 8, 2022

@mrniko

You fixed the message connection but neglected the pubsub connection.
PubSubConnectionPool overrides poll and releaseConnection, which will not call incUsage and decUsage.

When acquire create new pubsub connection, incUsage in ConnectionPool without checking PubSubConnectionPool will lead usage to become 1.
Connection pinging will also work incorrectly in this case.

【PubSubConnectionPool】
image

【pollConnection】
image

【releaseConnection】
image

【pollSubscribeConnection】
image

【releaseSubscribeConnection】
image

@mrniko mrniko reopened this Aug 8, 2022
mrniko pushed a commit that referenced this issue Aug 23, 2022
@mrniko
Copy link
Member

mrniko commented Aug 23, 2022

@LYFKiririn

Fixed. Thanks for code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants