Skip to content

Enforce rollback for each connection when shutdown occurs#5659

Merged
sougou merged 11 commits intovitessio:masterfrom
planetscale:sa-fix-5553
Jan 15, 2020
Merged

Enforce rollback for each connection when shutdown occurs#5659
sougou merged 11 commits intovitessio:masterfrom
planetscale:sa-fix-5553

Conversation

@saifalharthi
Copy link
Copy Markdown
Contributor

@saifalharthi saifalharthi commented Jan 6, 2020

This fixes #5553

Signed-off-by: Saif Alharthi <saif@saifalharthi.me>
Signed-off-by: Saif Alharthi <saif@saifalharthi.me>
Signed-off-by: Saif Alharthi <saif@saifalharthi.me>
Signed-off-by: Saif Alharthi <saif@saifalharthi.me>
@saifalharthi saifalharthi marked this pull request as ready for review January 6, 2020 14:28
@saifalharthi saifalharthi requested a review from sougou as a code owner January 6, 2020 14:28
Signed-off-by: Saif Alharthi <saif@saifalharthi.me>
Signed-off-by: Saif Alharthi <saif@saifalharthi.me>
Signed-off-by: Saif Alharthi <saif@saifalharthi.me>
@saifalharthi saifalharthi changed the title [WIP] Enforce rollback for each connection when shutdown occurs Enforce rollback for each connection when shutdown occurs Jan 13, 2020
Signed-off-by: Saif Alharthi <saif@saifalharthi.me>
}
}

func rollbackAtShutdown() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need to hold mutex here, because you're accessing connections. However, you'll run into a deadlock because ConnectionClosed also obtains the mutex.

I recommend implementing a CloseAllConnections instead of ConnectionClosed, that obtains the mutex and closes all conections in a loop.

And then, you can change servenv.OnClose(rollbackAtShutdown) -> servenv.OnClose(vtgateHandler.CloseAllConnections)

The previous approach we tried led to some race conditions.
This new approach of just hard-closing the connection and
then wait for the cleanup to happen from that effect feels
more stable.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
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.

VTGate does not rollback in-flight transactions when shut down

2 participants