Juju Charms Collection
postgresql package

Merge lp://staging/~jaywink/charms/trusty/postgresql/swiftwal_missing_functionality into lp://staging/charms/trusty/postgresql

  1. Trusty Tahr (14.04)
  2. swiftwal_missing_functionality
  3. Merge into trunk
Proposed by Jason Robinson
Status: Rejected
Rejected by: Charles Butler
Proposed branch: lp://staging/~jaywink/charms/trusty/postgresql/swiftwal_missing_functionality
Merge into: lp://staging/charms/trusty/postgresql
Diff against target: 180 lines (+53/-17)
5 files modified
TODO (+2/-0)
config.yaml (+13/-9)
hooks/hooks.py (+36/-0)
templates/postgres.cron.tmpl (+1/-7)
templates/swiftwal.conf.tmpl (+1/-1)
To merge this branch: bzr merge lp://staging/~jaywink/charms/trusty/postgresql/swiftwal_missing_functionality
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Disapprove
Review via email: mp+235394@code.staging.launchpad.net

Description of the change

Enable missing functionality for SwiftWAL backups. The settings we're marked EXPERIMENTAL but afaict the functionality was not finished.

Deprecate settings 'swiftwal_log_shipping', reason: --xlog option should always be passed to postgresql backup tool according to Swiftwal documentation. Otherwise backups could be unrecoverable.

Rename setting swiftwal_container_prefix to swiftwal_container. Swiftwal expects to receive full container name to backup to. I left both these in the config for now?

Add packages required by SwiftWAL, install swiftwal.conf file from template and tune crontab template to work with swiftwal.

NOTE! NOT ready for merging. Still needs some further testing, for example restore case (and maybe tests too?), but submitting for early review for comments, whether these fixes could be included, and what clearly needs to be fixed.

To post a comment you must log in.
Revision history for this message
Stuart Bishop (stub) wrote :

Hi.

I've had a branch finishing off this behavior in the review queue for the last few weeks - there is some traction now and it should land soon. I've also got wal-e support done, which I think will be a better option in the long term than SwiftWAL (there are a few subtle differences, but wal-e is supported by someone else and much better known since I've never announced SwiftWAL or moved it to a non-beta version number).

This is the original merge proposal that fixes up SwiftWAL:

https://code.launchpad.net/~stub/charms/precise/postgresql/bug-1276024-fix-log-shipping/+merge/225324

This is the wal-e one:

https://code.launchpad.net/~stub/charms/precise/postgresql/wal-e/+merge/225807

This is the one that this work will likely land as, since the reviewers seem to want one but smoosh of a merge proposal rather than the work broken up into manageable chunks:

https://code.launchpad.net/~stub/charms/precise/postgresql/integration/+merge/233666

Revision history for this message
Stuart Bishop (stub) wrote :

I'm going to have to reject this branch. It is one big conflict with the in-progress work that really needs to land due to other dependent work, going back several weeks.

review: Disapprove
Revision history for this message
Jason Robinson (jaywink) wrote :

Thanks for the review Stuart. I did have a look at the other branches to see if this was being worked on, and also babbled a bit on IRC, but somehow I missed your branches. No worries, got to play around with some of the charmhelpers stuff and learn some more Juju hook internals. It seems I also missed the stuff relating to handling clusters somehow.

Hoping to see your branches merged soon, would love to have this feature implemented for my services running on OpenStack using this charm.

Revision history for this message
Charles Butler (lazypower) wrote :

based on stubs comments above, listing this MP as rejected.

Thanks for the submission Jaywink.

Unmerged revisions

102. By Jason Robinson

Enable missing functionality for SwiftWAL backups.
Deprecate settings 'swiftwal_log_shipping', reason: --xlog option should always be
passed to postgresql backup tool according to Swiftwal documentation. Otherwise backups
could be unrecoverable.
Rename setting swiftwal_container_prefix to swiftwal_container. Swiftwal expects to
receive full container name to backup to.
Add packages required by SwiftWAL, install swiftwal.conf file from template and tune
crontab template to work with swiftwal.

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

to all changes: