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.

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

fix spurious errors when closing inactive connections #408

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

demmer
Copy link
Collaborator

@demmer demmer commented Jun 11, 2024

Description

When a connection is closed or reset before it is properly established, the existing ::getSession will log a spurious error since the connection has no attributes to extract for the pool type.

Fix this by refactoring so that when closing a connection we only act on the session object if one already existed, i.e. the conn was used before.

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on the CI
  • Documentation was added or is not required

Deployment Notes

When a connection is closed or reset before it is properly established, the
existing ::getSession will log a spurious error since the connection has no
attributes to extract for the pool type.

Fix this by refactoring so that when closing a connection we only act on the
session object if one already existed, i.e. the conn was used before.
func (ph *proxyHandler) closeSession(ctx context.Context, c *mysql.Conn) {
session, _ := c.ClientData.(*vtgateconn.VTGateSession)
if session == nil {
return // no active session
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously when this was in ::getSession, it would create the session, which would trigger the spurious error if the Conn had no attributes because it wasn't properly established in the first place.

Copy link
Collaborator

@rjlaine rjlaine left a comment

Choose a reason for hiding this comment

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

That makes sense and explains the behavior.

@demmer demmer merged commit 8f114fa into vtgateproxy Jun 11, 2024
152 of 241 checks passed
@demmer demmer deleted the vtgateproxy-fix-close-session branch June 11, 2024 16:40
dedelala pushed a commit that referenced this pull request Jun 12, 2024
When a connection is closed or reset before it is properly established, the
existing ::getSession will log a spurious error since the connection has no
attributes to extract for the pool type.

Fix this by refactoring so that when closing a connection we only act on the
session object if one already existed, i.e. the conn was used before.
dedelala pushed a commit that referenced this pull request Jul 30, 2024
When a connection is closed or reset before it is properly established, the
existing ::getSession will log a spurious error since the connection has no
attributes to extract for the pool type.

Fix this by refactoring so that when closing a connection we only act on the
session object if one already existed, i.e. the conn was used before.
dedelala pushed a commit that referenced this pull request Sep 9, 2024
When a connection is closed or reset before it is properly established, the
existing ::getSession will log a spurious error since the connection has no
attributes to extract for the pool type.

Fix this by refactoring so that when closing a connection we only act on the
session object if one already existed, i.e. the conn was used before.
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.

2 participants