1991santhu commented on PR #11678: URL: https://github.com/apache/seatunnel/pull/11678#issuecomment-5218522988
Thanks — the logging point is the one that mattered, and you're right about why. Replacements now log at WARN, rate limited to one message a minute, with the count since the last message. That's what makes the two cases distinguishable: occasional idle recovery produces an isolated line with a count of 1, while a misconfigured `connectionTestQuery` or a server-side connection limit produces a line every minute with a large count. Only an actual replacement logs — the first borrow for an index doesn't, so a healthy job stays quiet. Added a note at `isConnectionValid` that it isn't a pure predicate. I'd rather it be stated at the method than left for the next reader to work out from two hops away. Also pinned the validation timeout in tests, which was only covered by a loose matcher before: the pool's own `validationTimeout` when set, the 5000ms fallback when it isn't, and the one-second floor. That last one is the case worth having — a sub-second setting truncates to zero seconds, and `isValid(0)` means "no timeout" rather than "fail fast", so the floor is load-bearing. I checked the first and third fail if the timeout resolution is bypassed. On the two you raised that I haven't changed: The `DsqlConnectionPoolManager` scope gap is real and I'd rather do it separately. It has the same unvalidated cache, and as you say its connections are additionally invalidated out-of-band by the token refresh, so the fix isn't identical and would need its own tests. Happy to open that as a follow-up once this lands. On prepared statements outliving a swapped connection — agreed on the analysis, and agreed it isn't worth changing here. The wasted flush attempt comes out of the retry budget rather than failing the job, and the alternative would mean invalidating statements from inside the pool manager, which would put it in the business of knowing about its callers. Statement of what I ran: `ConnectionPoolManagerTest` 9/9, full `connector-jdbc` module 926 tests with 0 failures. The 4 errors in the module run are Testcontainers failing to start containers because Docker wasn't running on my machine, not related to this change. -- 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]
