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

fix: DPE-6485 Manage mysqld directly #575

Merged
merged 12 commits into from
Feb 9, 2025
Merged

Conversation

paulomach
Copy link
Contributor

@paulomach paulomach commented Feb 4, 2025

Issue

Mysqld_safe managed mysqld fails to restart when managed by pebble, due to mysqld_safe trapping SIGTERM, sent by pebble.

Solution

  • ditch mysqld_usage, manage mysqld directly
  • override configure_instance method to control restart within charm code, as mysql does not recognize pebble as a valid service manager
  • refactor restart method to ditch rolling ops event defer and reuse unit recovery method (from upgrade code)

Merge after PR#607
fix #569

Copy link
Contributor

@shayancanonical shayancanonical left a comment

Choose a reason for hiding this comment

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

less rolling ops is good!

Comment on lines +455 to +457
if self.app.planned_units() == 1:
self._mysql.reboot_from_complete_outage()
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in this comment. Maybe we can de-indent the whole else block 🤔

Copy link
Contributor

@taurus-forever taurus-forever left a comment

Choose a reason for hiding this comment

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

Good to have rid of mysqld_safe. Do we remember why it was introduced the first place (to recheck regression there)?

AFAIR, in #179 the InnoDB were not able to recover in the corner cases as RESTART was not working inside mysql/shell? Is it now working?

@shayancanonical
Copy link
Contributor

Good to have rid of mysqld_safe. Do we remember why it was introduced the first place (to recheck regression there)?

AFAIR, in #179 the InnoDB were not able to recover in the corner cases as RESTART was not working inside mysql/shell? Is it now working?

I believe that the RESTART still will not work inside mysql-shell. That is why I assumed that we are manually restarting during initial setup. however, it may be a problem in upgrades if we expect mysql-shell to restart mysqld but cannot

tests consistently fails on CI but work on my machine(tm), so this eliminates any differences from the env
@paulomach paulomach merged commit 66bfe57 into main Feb 9, 2025
101 of 102 checks passed
@paulomach paulomach deleted the fix/dpe-6485-restart-fix branch February 9, 2025 01:12
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.

Failed restart during update of memory and connection config with active connections
5 participants