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

Close Idle reserved connections and Rollback Idle transactions #6552

Merged
merged 3 commits into from
Aug 18, 2020

Conversation

harshit-gangal
Copy link
Member

@harshit-gangal harshit-gangal commented Aug 11, 2020

Currently reserved connections and transactions both are killed after a specific connection timeout from the time of its first use.
This change make use of last connection used time to see if the connection is idle for a certain time.

The current modification changes the logic for both reserved and transaction connection to use last used time. Also, the transaction connections will call rollback rather than closing the underlying connection.

Open Discussion Point:

  1. Should transaction connection killer still continue to use created time to identify the idle transactions or it can use the last used time?
  2. Should transaction connection killer still continue to close idle transaction or it can be changed to do rollback instead?

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@deepthi deepthi modified the milestone: 7.0.1 Aug 11, 2020
@systay
Copy link
Collaborator

systay commented Aug 12, 2020

I don’t think changing the behaviour of the transaction killer is a good idea. Long running transactions are problematic because they lock rows and resources for other users. the same does not really apply to long running reserved connections. I would suggest we keep transaction killing as-is, and only start killing reserved connections by last time used.

Have no opinions on if we should do a ROLLBACK or not.

@systay systay changed the title Close Idle tainted connections and Rollback Idle transactions Close Idle reserved connections and Rollback Idle transactions Aug 12, 2020
Copy link
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

Agree with @systay on the tx connection timing. That should continue to use start time.

systay and others added 2 commits August 14, 2020 12:33
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@harshit-gangal harshit-gangal marked this pull request as ready for review August 14, 2020 11:55
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.

5 participants