Skip to content

Conversation

@sjaakola
Copy link
Contributor

@sjaakola sjaakola commented Apr 4, 2023

background rollback did happen.
Background rollback should be skipped if the aborting happens due to KILL command issued by user. Some KILL signals, like KILL CONNECTION, wake up the victim too early so that background rollback could happen in parallel with the victim waking up and continuing execution.

@sjaakola sjaakola requested a review from temeo April 4, 2023 10:15
* Perform a background rollback for a transaction.
*/
virtual void background_rollback(wsrep::client_state&) = 0;
virtual bool background_rollback(wsrep::client_state&) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document the meaning of return value in comment above.

server_service_.background_rollback(client_state_);
/* if background rollback is skipped, reset rollbacker activity */
if (server_service_.background_rollback(client_state_))
client_state_.set_rollbacker_active(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling set_rollbacker_active() should be done with lock held to avoid races.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it is possible that the client session has reached client_state::do_wait_rollback_complete_and_acquire_ownership() while the mutex was unlocked above. In order to wake up the client, the code should also notify the client_state via client_state_.cond_ to wake up the thread which hosts the client session.

{
client_state.before_rollback();
client_state.after_rollback();
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I do not understand but when this method can return true ?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is for unit testing, so it does not necessarily implement all the same behavior as real application.

@sjaakola sjaakola requested a review from temeo April 11, 2023 18:15
background rollback did happen.
Background rollback should be skipped if the aborting happens due to KILL command issued by user.
Some KILL signals, like KILL CONECTION, wake up the victim too early so that background rollback
could happen in parallel with the victim waking up and continuing execution.
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.

4 participants