-
Notifications
You must be signed in to change notification settings - Fork 60
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.
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
pool: add Instance info to ConnectionInfo #429
Conversation
6c9c60e
to
cfdb88c
Compare
cfdb88c
to
453357f
Compare
2d27324
to
bcdf9c7
Compare
There was a problem hiding this 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! Left a couple of minor comments below.
A commit message looks very long. Please, format it to 72 characters per line. Reference: https://www.tarantool.io/en/doc/latest/contributing/developer_guidelines/ |
bcdf9c7
to
ae6bfb4
Compare
Instance info in ConnectionInfo enables pool state monitoring and dynamic tarantool topology tracking.
ae6bfb4
to
1eb799d
Compare
tCases = append(tCases, makeInstances([]string{ | ||
servers[0], | ||
servers[1], | ||
servers[3]}, | ||
connOpts)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tCases = append(tCases, makeInstances([]string{ | |
servers[0], | |
servers[1], | |
servers[3]}, | |
connOpts)) | |
tCases = append(tCases, makeInstances([]string{ | |
servers[0], | |
servers[1], | |
servers[3], | |
}, connOpts)) |
Nit, up to you.
The Instance info has been added to ConnectionInfo, which is necessary to monitor the current state of the pool. This should help implement dynamic tracking of tarantool topology changes.
What has been done? Why? What problem is being solved?
I didn't forget about (remove if it is not applicable):
Related issues: