Skip to content

#389: add some logging for error of ConnectionPool.tryConnect #390

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

Maximilan4
Copy link

Description at: #389
I added some logging for ConnectionPool.tryConnect err result.

Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Just few non-critical notes:

  1. Please, fix the review comment.
  2. Please, add the changelog entry here:

Something like this:

## Added
- Log messages if the pool unable to connect to an instance (#390)
  1. Please, squash commits into one commit and update the commit message. It could be something like:
pool: add log messages on connect error

I added some logging for ConnectionPool.tryConnect err result.

Part of #389

It will help us to have commits in the same style.

@oleg-jukovec oleg-jukovec requested a review from DerekBum March 25, 2024 12:09
Add err log to ConnectionPool.Add() in case, when unable to establish
connection and ctx is not canceled; also added logs for error case of
ConnectionPool.tryConnect() calls in ConnectionPool.controller() and ConnectionPool.reconnect()

Part of tarantool#389
Copy link
Collaborator

@oleg-jukovec oleg-jukovec 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!

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! Thanks for the patch!

@oleg-jukovec oleg-jukovec merged commit edde459 into tarantool:master Mar 26, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants