Merge lp://qastaging/~mariosplivalo/charms/trusty/percona-cluster/lp1366997-workaround into lp://qastaging/~openstack-charmers-archive/charms/trusty/percona-cluster/next

Proposed by Mario Splivalo
Status: Merged
Merged at revision: 46
Proposed branch: lp://qastaging/~mariosplivalo/charms/trusty/percona-cluster/lp1366997-workaround
Merge into: lp://qastaging/~openstack-charmers-archive/charms/trusty/percona-cluster/next
Diff against target: 87 lines (+27/-14)
3 files modified
config.yaml (+12/-8)
hooks/percona_hooks.py (+4/-1)
templates/my.cnf (+11/-5)
To merge this branch: bzr merge lp://qastaging/~mariosplivalo/charms/trusty/percona-cluster/lp1366997-workaround
Reviewer Review Type Date Requested Status
Billy Olsen Approve
charmers Pending
Edward Hope-Morley Pending
Review via email: mp+249392@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2015-02-11.

Description of the change

This MP adds another percona-cluster charm config option, lp1366997-workaround, which adds a specific configuration to mysql so that primary key bug is avoided.

To post a comment you must log in.
Revision history for this message
Edward Hope-Morley (hopem) wrote : Posted in a previous version of this proposal

Mario gonna retarget to /next. Please backport to stable once merged.

Revision history for this message
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal

From my understanding this config flag option 'lp1366997-workaround' is a flag which will be short-lived and hopefully removed if the upstream fix gets backported. I think it'd be best if we can find a solution which avoids the need for future deprecation.

If we can, I think it my be a good idea to have a flexible option which allows other options to be defined in a configuration file in the conf.d directory. This allows us to address this use case and possible future use cases without having to do additional code change with the convenient side-effect of avoiding the option deprecation/removal.

At the very least I think the config option should be renamed to convey a better indicator of what is addressing (symptom even) rather than the lpbug-workaround.

One other possible option is to simply enable this workaround all the time within the template. I'm not entirely sure of what the side-effects are of enabling these options so that may be the least desirable due to inadvertent risk (which is why it is toggled in the first place).

review: Needs Fixing
Revision history for this message
Billy Olsen (billy-olsen) wrote :

After discussion, the config option name is acceptable although I'm still not a fan.

Otherwise, code looks fine. I'll approve and merge into the /next branches for now. You can work on a backport.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches