Merge ~barryprice/cassandra-charm/+git/cassandra-charm:master into cassandra-charm:master

[Barry Price]

Add file_cache_size_in_mb option, alphabetise keys for readability in a couple of places

[Stuart Bishop (stub)]

Looks good!

review: Approve

13f11d0... by Barry Price on 2018-04-18

More pythonic value check, Flake8 fixes

diff --git a/config.yaml b/config.yaml
index 5ed5cf3..380b68c 100644
--- a/config.yaml
+++ b/config.yaml
@@ -125,7 +125,7 @@ options:
         description: >
             Set kernel io scheduler for persistent storage.
             https://www.kernel.org/doc/Documentation/block/switching-sched.txt
- 
+
 ##     units-to-update:
 ##         type: string
 ##         default: "all"
@@ -162,12 +162,12 @@ options:
         description: >
             The pct of heap used to trigger a nagios critcal alert
     nagios_disk_warn_pct:
-        default: 50 
+        default: 50
         type: int
         description: >
             The pct of data disk used to trigger a nagios warning
     nagios_disk_crit_pct:
-        default: 25 
+        default: 25
         type: int
         description: >
             The pct of data disk used to trigger a nagios critcal alert
@@ -202,6 +202,18 @@ options:
             another. All Cassandra services you wish to cluster together
             must have the same cluster_name. This setting cannot be changed
             after service deployment.
+    file_cache_size_in_mb:
+        type: int
+        default: 0
+        description: >
+            Maximum memory to use for sstable chunk cache and buffer pooling.
+            32MB of this are reserved for pooling buffers, the rest is used as
+            an cache that holds uncompressed sstable chunks. Defaults to the
+            smaller of 1/4 of heap or 512MB. This pool is allocated off-heap,
+            so is in addition to the memory allocated for heap. The cache also
+            has on-heap overhead which is roughly 128 bytes per chunk (i.e.
+            0.2% of the reserved size if the default 64k chunk size is used).
+            Memory is only allocated when needed.
     partitioner:
         default: Murmur3Partitioner
         type: string
@@ -212,7 +224,7 @@ options:
         type: int
         default: 256
         description: Number of tokens per node.
-            
+
     # Topology of the service in the cluster.
     datacenter:
         type: string
@@ -220,7 +232,7 @@ options:
         description: >
             The node's datacenter used by the endpoint_snitch. e.g. "DC1".
             It cannot be changed after service deployment.
-    rack: 
+    rack:
         type: string
         default: ""
         description: >
@@ -229,7 +241,7 @@ options:
             It defaults to the service name. Cassandra will store replicated
             data in different racks whenever possible.
 
-            
+
     # Networking options.
     native_transport_port:
         type: int
@@ -282,7 +294,7 @@ options:
             - CassandraAuthorizer stores permissions in
               system_auth.permissions table.
 
-   
+
     # Tuning options.
     compaction_throughput_mb_per_sec:
         type: int
@@ -301,8 +313,8 @@ options:
         description: >
             Throttles all outbound streaming file transfers on nodes to the
             given total throughput in Mbps. This is necessary because Cassandra
-            does mostly sequential IO when streaming data during bootstrap or 
-            repair, which can lead to saturating the network connection and 
+            does mostly sequential IO when streaming data during bootstrap or
+            repair, which can lead to saturating the network connection and
             degrading rpc performance. When unset, the default is 200 Mbps
             or 25 MB/s. 0 to disable throttling.
 
diff --git a/hooks/actions.py b/hooks/actions.py
index 3625d9a..5be6325 100644
--- a/hooks/actions.py
+++ b/hooks/actions.py
@@ -48,27 +48,28 @@ UNCHANGEABLE_KEYS = set(['cluster_name', 'datacenter', 'rack', 'edition'])
 # If any of these config items are changed, Cassandra needs to be
 # restarted and maybe remounted.
 RESTART_REQUIRED_KEYS = set([
-    'data_file_directories',
+    'authenticator',
+    'authorizer',
     'commitlog_directory',
-    'saved_caches_directory',
-    'storage_port',
-    'ssl_storage_port',
+    'compaction_throughput_mb_per_sec',
+    'data_file_directories',
+    'file_cache_size_in_mb',
+    'heap_newsize',
+    'jre',
     'listen_interface',
-    'rpc_port',
+    'max_heap_size',
     'native_transport_port',
-    'rpc_interface',
-    'partitioner',
     'num_tokens',
-    'max_heap_size',
-    'heap_newsize',
-    'authenticator',
-    'authorizer',
-    'compaction_throughput_mb_per_sec',
+    'partitioner',
+    'private_jre_url',
+    'rpc_interface',
+    'rpc_port',
+    'saved_caches_directory',
+    'ssl_storage_port',
+    'storage_port',
     'stream_throughput_outbound_megabits_per_sec',
-    'tombstone_warn_threshold',
     'tombstone_failure_threshold',
-    'jre',
-    'private_jre_url'])
+    'tombstone_warn_threshold',])
 
 ALL_CONFIG_KEYS = UNCHANGEABLE_KEYS.union(RESTART_REQUIRED_KEYS)
 
diff --git a/hooks/helpers.py b/hooks/helpers.py
index c71f9f7..1e666f3 100644
--- a/hooks/helpers.py
+++ b/hooks/helpers.py
@@ -877,14 +877,21 @@ def configure_cassandra_yaml(overrides={}, seeds=None):
     # Most options just copy from config.yaml keys with the same name.
     # Using the same name is preferred to match the actual Cassandra
     # documentation.
-    simple_config_keys = ['cluster_name', 'num_tokens',
-                          'partitioner', 'authorizer', 'authenticator',
+    simple_config_keys = ['authenticator', 'authorizer', 'cluster_name',
                           'compaction_throughput_mb_per_sec',
+                          'file_cache_size_in_mb', 'native_transport_port',
+                          'num_tokens', 'partitioner', 'rpc_port',
+                          'ssl_storage_port' 'storage_port',
                           'stream_throughput_outbound_megabits_per_sec',
-                          'tombstone_warn_threshold',
                           'tombstone_failure_threshold',
-                          'native_transport_port', 'rpc_port',
-                          'storage_port', 'ssl_storage_port']
+                          'tombstone_warn_threshold']
+
+    # file_cache_size_in_mb defaults to 0 in YAML, because int values need
+    # an int default - but if left as default, let cassandra figure it out
+    if (config.get('file_cache_size_in_mb') is None
+            or config.get('file_cache_size_in_mb') <= 0):
+        simple_config_keys.remove('file_cache_size_in_mb')
+
     cassandra_yaml.update((k, config[k]) for k in simple_config_keys)
 
     seeds = ','.join(seeds or get_seed_ips())  # Don't include whitespace!
diff --git a/tests/test_helpers.py b/tests/test_helpers.py
index 0d78568..f52a021 100755
--- a/tests/test_helpers.py
+++ b/tests/test_helpers.py
@@ -1016,6 +1016,7 @@ class TestHelpers(TestCaseBase):
         get_cassandra_version.return_value = '2.0'
         hookenv.config().update(dict(num_tokens=128,
                                      cluster_name='test_cluster_name',
+                                     file_cache_size_in_mb=768,
                                      partitioner='test_partitioner'))
 
         seed_ips.return_value = ['10.20.0.1', '10.20.0.2', '10.20.0.3']
@@ -1042,6 +1043,7 @@ class TestHelpers(TestCaseBase):
                 cluster_name: test_cluster_name
                 authenticator: PasswordAuthenticator
                 num_tokens: 128
+                file_cache_size_in_mb: 768
                 partitioner: test_partitioner
                 listen_address: 10.20.0.1
                 rpc_address: 0.0.0.0
@@ -1087,6 +1089,7 @@ class TestHelpers(TestCaseBase):
         get_cassandra_version.return_value = '2.0'
         hookenv.config().update(dict(num_tokens=128,
                                      cluster_name='test_cluster_name',
+                                     file_cache_size_in_mb=768,
                                      partitioner='test_partitioner'))
 
         seed_ips.return_value = ['10.20.0.1', '10.20.0.2', '10.20.0.3']
@@ -1115,6 +1118,7 @@ class TestHelpers(TestCaseBase):
                 cluster_name: test_cluster_name
                 authenticator: PasswordAuthenticator
                 num_tokens: 128
+                file_cache_size_in_mb: 768
                 partitioner: test_partitioner
                 listen_address: 10.20.0.1
                 rpc_address: 0.0.0.0
@@ -1158,6 +1162,7 @@ class TestHelpers(TestCaseBase):
                                       yaml_file, get_cassandra_version):
         get_cassandra_version.return_value = '2.1'
         hookenv.config().update(dict(num_tokens=128,
+                                     file_cache_size_in_mb=768,
                                      cluster_name='test_cluster_name',
                                      partitioner='test_partitioner'))
 
@@ -1185,6 +1190,7 @@ class TestHelpers(TestCaseBase):
                 cluster_name: test_cluster_name
                 authenticator: PasswordAuthenticator
                 num_tokens: 128
+                file_cache_size_in_mb: 768
                 partitioner: test_partitioner
                 listen_address: 10.20.0.1
                 rpc_address: 0.0.0.0