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.

Sign up for GitHub

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

Jump to bottom

Default maxLifeTime value result in immediate eviction of all connections #129

Closed
bezmax opened this issue Jun 21, 2021 · 4 comments · Fixed by #155
Closed

Default maxLifeTime value result in immediate eviction of all connections #129

bezmax opened this issue Jun 21, 2021 · 4 comments · Fixed by #155
Labels
status: ideal-for-contribution An issue that a contributor can help us with type: bug A general bug
Milestone
0.9.1.RELEASE

Comments

@bezmax
Copy link

bezmax commented Jun 21, 2021

Bug Report

The default for maxLifeTime is defined as NO_TIMEOUT = Duration.ofNanos(-1), which is supposed to mean "No timeout". However, the current implementation of actual evectionPredicate, attempts to do toMillis conversion, which results in 0, thus resulting in the if (maxIdleTimeMills == 0 || maxLifeTimeMillis == 0) always returning true.

Possible Solution

Replace maxLifeTimeMillis == 0 with maxLifeTime.isZero().
@mp911de mp911de added type: bug A general bug and removed type: bug A general bug labels Jun 22, 2021
@mp911de
Copy link
Member

mp911de commented Jun 22, 2021

Care to submit a pull request to fix the problem?

@bezmax
Copy link
Author

bezmax commented Jun 22, 2021

Sure, will do!

@mp911de mp911de added the status: ideal-for-contribution An issue that a contributor can help us with label Jun 22, 2021
@patriknw patriknw mentioned this issue Jan 26, 2022
Merged
patriknw added a commit to patriknw/r2dbc-pool that referenced this issue Feb 5, 2022
@patriknw
fix immedediate eviction for default maxLifeTime, r2dbc#129
48767b5
patriknw added a commit to patriknw/r2dbc-pool that referenced this issue Feb 5, 2022
@patriknw
fix immedediate eviction for default maxLifeTime, r2dbc#129
0ad476e 
Signed-off-by: Patrik Nordwall <patrik.nordwall@gmail.com>
@patriknw patriknw mentioned this issue Feb 5, 2022
Merged
patriknw added a commit to patriknw/r2dbc-pool that referenced this issue Feb 5, 2022
@patriknw
fix immediate eviction for default maxLifeTime, r2dbc#129
3a052b9 
Signed-off-by: Patrik Nordwall <patrik.nordwall@gmail.com>
@GabrielCalin
Copy link

GabrielCalin commented May 3, 2022 •
edited
Loading

Just a friendly ping, could someone merge this PR?
The fact is that this is a major bug at the moment, as the pool behaves poorly when maxLifeTime is not explicitly set.

@Inego
Copy link

Inego commented Jun 7, 2022

Current workaround: set spring.r2dbc.pool.max-life-time to -1.

@mp911de mp911de added this to the 0.9.1.RELEASE milestone Jun 22, 2022
mp911de pushed a commit that referenced this issue Jun 22, 2022
@patriknw
Fix immediate eviction for default (no) maxLifeTime
0fa83a3 
[resolves #129][#155]
Signed-off-by: Patrik Nordwall <patrik.nordwall@gmail.com>
mp911de pushed a commit that referenced this issue Jun 22, 2022
@patriknw @mp911de
Fix immediate eviction for default (no) maxLifeTime
0f16586 
[resolves #129][#155]
Signed-off-by: Patrik Nordwall <patrik.nordwall@gmail.com>
@breun breun mentioned this issue Aug 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
status: ideal-for-contribution An issue that a contributor can help us with type: bug A general bug
Projects
None yet
Milestone
0.9.1.RELEASE
Development

Successfully merging a pull request may close this issue.

Fix immediate eviction for default maxLifeTime patriknw/r2dbc-pool
4 participants
@mp911de @Inego @GabrielCalin @bezmax