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

pump: prevent pump progress from an exit with 0 if the pump become offline #769

Merged
merged 1 commit into from
Aug 16, 2019

Conversation

weekface
Copy link
Contributor

What problem does this PR solve?

prevent pump progress from an exit with 0 if the pump become offline
, because it will be online again if the pod get restarted by statefulset.

What is changed and how does it work?

Check List

Tests

Code changes

  • Has Helm charts change

Side effects

Related changes

  • Need to cherry-pick to the release branch

Does this PR introduce a user-facing change?:

prevent pump progress from an exit with 0 if the pump become `offline`

@weekface
Copy link
Contributor Author

/run-e2e-in-kind

Copy link
Member

@tennix tennix left a comment

Choose a reason for hiding this comment

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

LGTM

@weekface
Copy link
Contributor Author

/run-e2e-in-kind

if [ $? == 0 ]; then
echo $(date -u +"[%Y/%m/%d %H:%M:%S.%3N %:z]") "pump offline, please delete my pod"
tail -f /dev/null
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you be sure that if the exit code is 0, it must be pump offline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This script ensures that the pump container would not exit normally, it must be killed by kubelet.

Copy link
Member

Choose a reason for hiding this comment

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

The pause state also ends with exit code 0, but anyway the pod needs to be running.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if the container get restarted?

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, due to node failure or eviction

Copy link
Member

Choose a reason for hiding this comment

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

If container restarted, it will be running again, users have to issue binlogctl again to offline the pump. And if the pod is evicted for disk or node failure, then that data may be lost because pump data is not replicated.

Copy link
Contributor

@aylei aylei left a comment

Choose a reason for hiding this comment

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

LGTM

@weekface weekface merged commit 2e70a73 into pingcap:master Aug 16, 2019
@weekface weekface deleted the fix-pump-offline branch August 16, 2019 03:17
@sre-bot
Copy link
Contributor

sre-bot commented Aug 16, 2019

cherry pick to release-1.0 in PR #778

yahonda pushed a commit that referenced this pull request Dec 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants