Merge lp:~bellini666/charms/precise/postgresql/extra_options into lp:charms/postgresql

[Thiago Bellini]

Some extra options we set on one of our postgresql clusters.

Those are useful to tune the cluster performance a bit.

[Stuart Bishop (stub)]

Thanks for this.

I think that, apart from checkpoint_timeout, the new options should be 'type: boolean' with a default matching the PostgreSQL default, the same as existing options like 'autovacuum' and 'log_checkpoints'. This will also let you remove the conditionals from the template.

I think that the fsync option should come with a warning, something along the lines of 'If fsync is turned off, database failures are likely to involve database corruption and require recreating the unit'. This is of course made clear in the PostgreSQL documentation, but I expect some juju users won't be reading that.

None of these options conflict with the recent landing of pgtune support, which is good.

I'm happy to land this branch with these changes, and they are simple enough that I can make the changes for you if you would rather.

review: Needs Fixing

[Thiago Bellini (bellini666)]

> Thanks for this.
>
> I think that, apart from checkpoint_timeout, the new options should be 'type:
> boolean' with a default matching the PostgreSQL default, the same as existing
> options like 'autovacuum' and 'log_checkpoints'. This will also let you remove
> the conditionals from the template.

Yes, I think it's better too. I didn't know that postgresql accepts true/false too. I though it should be on/off only.

But I'm still keeping the conditionals on the template to follow the convention there (all other options on the template, even boolean ones, are using the conditional)

>
> I think that the fsync option should come with a warning, something along the
> lines of 'If fsync is turned off, database failures are likely to involve
> database corruption and require recreating the unit'. This is of course made
> clear in the PostgreSQL documentation, but I expect some juju users won't be
> reading that.

haha np, done!

>
> None of these options conflict with the recent landing of pgtune support,
> which is good.
>
> I'm happy to land this branch with these changes, and they are simple enough
> that I can make the changes for you if you would rather.

I'm committing the changes right now. Thank you!

[lp:~bellini666/charms/precise/postgresql/extra_options]

96. By Thiago Bellini on 2014-05-13

Allow to tune performance

Those options make it possible to tune the performance on postgresql.
Since they are empty, they will be left out of postgresql.conf unless
set by the user.

[Stuart Bishop (stub)]

All good. Landing.

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2014-05-07 04:49:06 +0000
+++ config.yaml	2014-05-13 21:06:27 +0000
@@ -287,6 +287,29 @@
     description: |
         in logfile segments, min 1, 16MB each.
         Ignored unless 'performance_tuning' is set to 'manual'.
+  checkpoint_timeout:
+    default: ""
+    type: string
+    description: |
+        Maximum time between automatic WAL checkpoints. range '30s-1h'.
+        If left empty, the default postgresql value will be used.
+  fsync:
+    type: boolean
+    default: True
+    description: |
+        Turns forced synchronization on/off. If fsync is turned off, database
+        failures are likely to involve database corruption and require
+        recreating the unit
+  synchronous_commit:
+    type: boolean
+    default: True
+    description: |
+        Immediate fsync after commit.
+  full_page_writes:
+    type: boolean
+    default: True
+    description: |
+        Recover from partial page writes.
   random_page_cost:
     default: 4.0
     type: float

=== modified file 'templates/postgresql.conf.tmpl'
--- templates/postgresql.conf.tmpl	2014-02-13 11:50:00 +0000
+++ templates/postgresql.conf.tmpl	2014-05-13 21:06:27 +0000
@@ -70,6 +70,18 @@
 {% if checkpoint_segments  != "" -%}
 checkpoint_segments = {{checkpoint_segments}}
 {% endif -%}
+{% if checkpoint_timeout  != "" -%}
+checkpoint_timeout = {{checkpoint_timeout}}
+{% endif -%}
+{% if fsync  != "" -%}
+fsync = {{fsync}}
+{% endif -%}
+{% if synchronous_commit  != "" -%}
+synchronous_commit = {{synchronous_commit}}
+{% endif -%}
+{% if full_page_writes  != "" -%}
+full_page_writes = {{full_page_writes}}
+{% endif -%}
 
 
 #------------------------------------------------------------------------------