Skip to content

Conversation

@mhamza15
Copy link

Description

This is a backport of vitessio#18530

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@mhamza15 mhamza15 self-assigned this Aug 13, 2025
Copilot AI review requested due to automatic review settings August 13, 2025 19:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a connection pool reopening mechanism by storing configuration and stats exporter references, allowing pools to be replaced when closed and reopened. This is a backport of a previous fix aimed at properly handling pool lifecycle management.

  • Stores pool configuration and stats exporter references for later reuse
  • Replaces the underlying connection pool when closing to enable clean reopening
  • Introduces proper pool state management with atomic operations

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
go/vt/vttablet/tabletserver/connpool/pool.go Stores config and stats exporter, replaces ConnPool on close
go/vt/dbconnpool/connection_pool.go Adds config storage and Close method that replaces the pool
go/pools/smartconnpool/pool_test.go Adds test for connection reopening behavior
go/pools/smartconnpool/pool.go Implements pool state management and separate StatsExporter

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@mhamza15 mhamza15 force-pushed the mhamza/backport-18530-v19 branch 2 times, most recently from 431ad94 to fd4ed7b Compare August 13, 2025 22:55
arthurschreiber and others added 9 commits August 14, 2025 14:42
Signed-off-by: Arthur Schreiber <[email protected]>
Signed-off-by: Arthur Schreiber <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
When closing the pool, there is a race condition between the closing of the `close`
channel in `CloseWithContext`, and the `maybeStarving` worker closing it. We'll remove
the close in `CloseWithContext` and allow the worker to close it on its next run.

Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
closing and normal operations

Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
@mhamza15 mhamza15 force-pushed the mhamza/backport-18530-v19 branch from fd4ed7b to 32f06d3 Compare August 14, 2025 18:42
@mhamza15 mhamza15 force-pushed the mhamza/backport-18530-v19 branch from d27a57e to bed0c14 Compare August 14, 2025 22:31
@mhamza15 mhamza15 closed this Sep 3, 2025
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.

4 participants