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

vrepl: vplayer must rollback on exit #5842

Merged
merged 1 commit into from
Feb 21, 2020

Conversation

sougou
Copy link
Contributor

@sougou sougou commented Feb 21, 2020

There were code paths where vplayer had an open transaction on
its dbclient connection, which got accidentally continued by
the copier. This caused production issues with some users.

The specific use case that was observed is:

  • vreplication performs catchup.
  • While it's in the middle of an apply, the context is canceled.
  • The copier has no work.
  • vreplication is asked to resume from the last saved position.
  • It replays the same statements already in the transaction.
  • This leads to dup key errors.

This fix conservatively rolls back dbclient before exiting.
The dbclient itself skips rollback if it's not in a transaction.
This change leads to relieable rollbacks where needed. At the
same time, there are no spurious rollbacks if we didn't start
a transaction.

As safety, I've added an extra rollback in the vreplicator loop
just in case the underlying functions accidentally leave an
incomplete transaction open.

Signed-off-by: Sugu Sougoumarane ssougou@gmail.com

There were code paths where vplayer had an open transaction on
its dbclient connection, which got accidentally continued by
the copier. This caused production issues with some users.

The specific use case that was observed is:
* vreplication performs catchup.
* While it's in the middle of an apply, the context is canceled.
* The copier has no work.
* vreplication is asked to resume from the last saved position.
* It replays the same statements already in the transaction.
* This leads to dup key errors.

This fix conservatively rolls back dbclient before exiting.
The dbclient itself skips rollback if it's not in a transaction.
This change leads to relieable rollbacks where needed. At the
same time, there are no spurious rollbacks if we didn't start
a transaction.

As safety, I've added an extra rollback in the vreplicator loop
just in case the underlying functions accidentally leave an
incomplete transaction open.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou sougou requested a review from deepthi February 21, 2020 20:16
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

LGTM

@sougou sougou merged commit eb94add into vitessio:master Feb 21, 2020
@sougou sougou deleted the ss-vrepl-bug-rollback branch February 21, 2020 21:07
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