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

pool: add a connection even on connection error #374

Merged
merged 1 commit into from
Jan 30, 2024
Merged

pool: add a connection even on connection error #374

merged 1 commit into from
Jan 30, 2024

Conversation

oleg-jukovec
Copy link
Collaborator

From a user's perspective, it is useful to add all target instances to the pool, even some that are not currently unavailable. This way the user don’t have to keep track of the list of actually added instances.

The patch make it possible.

Closes #372

DerekBum
DerekBum reviewed Jan 26, 2024
View reviewed changes
pool/connection_pool.go Outdated Show resolved Hide resolved
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-372-pool-connect-improve branch from 50dc5d0 to f42c2ed Compare January 26, 2024 16:57
DerekBum
DerekBum approved these changes Jan 26, 2024
View reviewed changes
Copy link

@DerekBum DerekBum left a comment

Choose a reason for hiding this comment

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

Lgtm!

DerekBum
DerekBum reviewed Jan 26, 2024
View reviewed changes
Copy link

@DerekBum DerekBum left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Seems that there is some data race for test TestConnectionHandlerOpenError. Maybe this happened because connPool continues to work after its creation (and not closed immediately due to an error as it was before).

@DerekBum DerekBum self-requested a review January 26, 2024 17:18
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-372-pool-connect-improve branch from f42c2ed to 74ae72d Compare January 26, 2024 17:58
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-372-pool-connect-improve branch 3 times, most recently from b66a2c8 to 305c761 Compare January 28, 2024 20:41
@oleg-jukovec
pool: add a connection even on connection error
13789ce 
From a user's perspective, it is useful to add all target instances
to the pool, even some that are not currently unavailable. This way
the user don’t have to keep track of the list of actually added
instances.

The patch make it possible.

Closes #372
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-372-pool-connect-improve branch from 305c761 to 13789ce Compare January 28, 2024 20:58
@oleg-jukovec oleg-jukovec requested review from DerekBum and askalt and removed request for DerekBum and askalt January 28, 2024 21:22
askalt
askalt approved these changes Jan 29, 2024
View reviewed changes
Copy link

@askalt askalt left a comment

Choose a reason for hiding this comment

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

Thank you for the patch!

@oleg-jukovec oleg-jukovec merged commit f02579a into master Jan 30, 2024
22 checks passed
@oleg-jukovec oleg-jukovec deleted the oleg-jukovec/gh-372-pool-connect-improve branch January 30, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@askalt askalt askalt approved these changes

@DerekBum DerekBum DerekBum approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pool: improve pool.Connect logic
3 participants
@oleg-jukovec @DerekBum @askalt