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

Providing JedisPool to be created with abandon config #2054

Closed
wants to merge 1 commit into from

Conversation

venukbh
Copy link

@venukbh venukbh commented Sep 5, 2019

There are a lot of discussions and forums indicating connection issues, maxing out over period of time, connections leakage and not returning to pool.

This abandon config will give the client an option to abandon connections which are not returned back in a specified time - making the connection pool steady and available for app all the time

@venukbh
Copy link
Author

venukbh commented Sep 20, 2019

@gkorland : Can you approve this PR. This PR has helped us fix an issue by cleaning the connections which are being left active due to various reasons at app level for life time

@venukbh
Copy link
Author

venukbh commented Sep 24, 2019

Hi @argvk Can you approve this PR. I do not see any use case to write a junit as I am just providing an additonal constructor. This change helps us to fix lot of consumers whose connections are getting stale over time and systems going bad.

@argvk
Copy link
Contributor

argvk commented Sep 24, 2019

hey @venukbh, I am not a maintainer, and haven't worked on this repo for a long time. You should reach out to the maintainers.

@venukbh venukbh force-pushed the jedis-pool-with-abandon-config branch 3 times, most recently from 7a0ac24 to b47e7f3 Compare December 4, 2019 19:44
@venukbh
Copy link
Author

venukbh commented Dec 4, 2019

@gkorland Can you take a look and approve. Even Redis enterprise team found it useful. Let me know of any thing to clarify

@venukbh venukbh force-pushed the jedis-pool-with-abandon-config branch from b47e7f3 to e11dc77 Compare December 4, 2019 20:02
@venukbh
Copy link
Author

venukbh commented Dec 5, 2019

@sazzad16 @karltinawi @ullenius @Talon876 : Can you please take a look at this and merge... This is much needed when the connections are not being released by the system

@sabir012
Copy link

Hi all,
Is there any action to merge this change to master? I also found the same issue with active connections.

@sazzad16
Copy link
Collaborator

@sabir012 @venukbh It is hard to accept a PR without tests even if ill-formatted codes are ignored.

@venukbh
Copy link
Author

venukbh commented Jan 14, 2020

@sazzad16 I do understand about the test cases importance. But I am just providing constructors, and I do not see any existing test cases for constructors as well.

@venukbh venukbh force-pushed the jedis-pool-with-abandon-config branch from e11dc77 to 266a2bc Compare February 3, 2020 16:09
@venukbh
Copy link
Author

venukbh commented Feb 3, 2020

@sazzad16 Can you please merge this pr? The change introduced helps a lot of clients using redis to get rid of the connection pool issues.

@karltinawi
Copy link
Contributor

hi @venukbh - sorry for the lateness in replying, I am not a maintainer either. Hope you get this merged soon..!

@venukbh
Copy link
Author

venukbh commented Feb 6, 2020

@sazzad16 @gkorland can you approve this?

@gkorland gkorland requested a review from sazzad16 February 11, 2020 11:58
@gkorland
Copy link
Contributor

@venukbh I think that instead of overloading another Constructor (and perhaps many more) we should just expose setAbandonedConfig in Pool.java

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

Successfully merging this pull request may close these issues.

7 participants