apache
TINKERPOP-2617 Clean up Java connection pool logic
Removed duplicated code for fetching a connection form the ConnectionPool.
Codecov Report
Merging #1709 (f0a5e76) into 3.5-dev (04f47d2) will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## 3.5-dev #1709 +/- ##
========================================
Coverage 63.51% 63.51%
========================================
Files 23 23
Lines 3601 3601
========================================
Hits 2287 2287
Misses 1136 1136
Partials 178 178
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 04f47d2...f0a5e76. Read the comment docs.
@divijvaidya do you have time to look at this? I think you have the most context on Java connection pooling.
Let me get to a first review by end of this week @simonz-bq . Meanwhile, it would be great if we could get some testing around the edge cases that you mentioned which aren't correctly handled in the current code.
Let me get to a first review by end of this week @simonz-bq . Meanwhile, it would be great if we could get some testing around the edge cases that you mentioned which aren't correctly handled in the current code.
@divijvaidya the additional edge cases logic was removed, but I forgot to update the initial commit comment. It was causing existing integration tests to fail and hang and investigation into how to fixing them proved to be difficult and complex for what was being considered as a easy win.
Regarding the ask for more tests, the logic did not change. This is a refactor and the functionality behaves similar to before and is ultimately just for the sake of reducing code duplication.
Originally I had made some minor logic adjustments that were supposed to improve the logic, but it lead to nebulous failures with existing tests, and how to fix them or write new ones is very esoteric. Investigating how to fix them exceeded a time box that we'd expected for what was supposed to be a simple code refactor so I had reverted the adjustments and left only the refactoring.
More work can definitely be done regarding the connection pooling, but I think doing so here would exceed the scope of the ticket, especially given that an attempt was made and it proved to be problematic and not straightforward.
