From 50cadac3f2ba7a5ba8da8469549c3f2aa847104a Mon Sep 17 00:00:00 2001 From: Matthew McGinn Date: Wed, 4 Jul 2018 00:35:15 -0400 Subject: [PATCH 1/2] Add shard_duration parameter when creating or altering retention policies Fixes #560 --- CHANGELOG.md | 1 + influxdb/client.py | 34 +++++++-- influxdb/tests/client_test.py | 12 ++- .../server_tests/client_test_with_server.py | 75 ++++++++++++++++++- 4 files changed, 110 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0791c2ea..b524a83e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Finally add a CHANGELOG.md to communicate breaking changes (#598) - Test multiple versions of InfluxDB in travis +- Add SHARD DURATION parameter to retention policy create/alter ### Changed - Update test suite to support InfluxDB v1.3.9, v1.4.2, and v1.5.4 - Fix performance degradation when removing NaN values via line protocol (#592) diff --git a/influxdb/client.py b/influxdb/client.py index 62d5a025..d11588d8 100644 --- a/influxdb/client.py +++ b/influxdb/client.py @@ -605,7 +605,8 @@ def drop_measurement(self, measurement): self.query("DROP MEASUREMENT {0}".format(quote_ident(measurement))) def create_retention_policy(self, name, duration, replication, - database=None, default=False): + database=None, + default=False, shard_duration="0s"): """Create a retention policy for a database. :param name: the name of the new retention policy @@ -624,12 +625,21 @@ def create_retention_policy(self, name, duration, replication, :type database: str :param default: whether or not to set the policy as default :type default: bool + :param shard_duration: the shard duration of the retention policy. + Durations such as 1h, 90m, 12h, 7d, and 4w, are all supported and + mean 1 hour, 90 minutes, 12 hours, 7 day, and 4 weeks, + respectively. Infinite retention is not supported. As a workaround, + specify a "1000w" duration to achieve an extremely long shard group + duration. Defaults to "0s", which is interpreted by the database + to mean the default value given the duration. + The minimum shard group duration is 1 hour. + :type shard_duration: str """ query_string = \ "CREATE RETENTION POLICY {0} ON {1} " \ - "DURATION {2} REPLICATION {3}".format( + "DURATION {2} REPLICATION {3} SHARD DURATION {4}".format( quote_ident(name), quote_ident(database or self._database), - duration, replication) + duration, replication, shard_duration) if default is True: query_string += " DEFAULT" @@ -637,8 +647,9 @@ def create_retention_policy(self, name, duration, replication, self.query(query_string) def alter_retention_policy(self, name, database=None, - duration=None, replication=None, default=None): - """Mofidy an existing retention policy for a database. + duration=None, replication=None, + default=None, shard_duration=None): + """Modify an existing retention policy for a database. :param name: the name of the retention policy to modify :type name: str @@ -657,15 +668,26 @@ def alter_retention_policy(self, name, database=None, :type replication: int :param default: whether or not to set the modified policy as default :type default: bool + :param shard_duration: the shard duration of the retention policy. + Durations such as 1h, 90m, 12h, 7d, and 4w, are all supported and + mean 1 hour, 90 minutes, 12 hours, 7 day, and 4 weeks, + respectively. Infinite retention is not supported. As a workaround, + specify a "1000w" duration to achieve an extremely long shard group + duration. + The minimum shard group duration is 1 hour. + :type shard_duration: str .. note:: at least one of duration, replication, or default flag should be set. Otherwise the operation will fail. """ query_string = ( "ALTER RETENTION POLICY {0} ON {1}" - ).format(quote_ident(name), quote_ident(database or self._database)) + ).format(quote_ident(name), + quote_ident(database or self._database), shard_duration) if duration: query_string += " DURATION {0}".format(duration) + if shard_duration: + query_string += " SHARD DURATION {0}".format(shard_duration) if replication: query_string += " REPLICATION {0}".format(replication) if default is True: diff --git a/influxdb/tests/client_test.py b/influxdb/tests/client_test.py index efdfb770..353bbebc 100644 --- a/influxdb/tests/client_test.py +++ b/influxdb/tests/client_test.py @@ -626,7 +626,7 @@ def test_create_retention_policy_default(self): self.assertEqual( m.last_request.qs['q'][0], 'create retention policy "somename" on ' - '"db" duration 1d replication 4 default' + '"db" duration 1d replication 4 shard duration 0s default' ) def test_create_retention_policy(self): @@ -646,7 +646,7 @@ def test_create_retention_policy(self): self.assertEqual( m.last_request.qs['q'][0], 'create retention policy "somename" on ' - '"db" duration 1d replication 4' + '"db" duration 1d replication 4 shard duration 0s' ) def test_alter_retention_policy(self): @@ -674,6 +674,14 @@ def test_alter_retention_policy(self): 'alter retention policy "somename" on "db" replication 4' ) + # Test alter shard duration + self.cli.alter_retention_policy('somename', 'db', + shard_duration='1h') + self.assertEqual( + m.last_request.qs['q'][0], + 'alter retention policy "somename" on "db" shard duration 1h' + ) + # Test alter default self.cli.alter_retention_policy('somename', 'db', default=True) diff --git a/influxdb/tests/server_tests/client_test_with_server.py b/influxdb/tests/server_tests/client_test_with_server.py index 701f72ac..c18aa328 100644 --- a/influxdb/tests/server_tests/client_test_with_server.py +++ b/influxdb/tests/server_tests/client_test_with_server.py @@ -526,13 +526,57 @@ def test_create_retention_policy(self): rsp ) + self.cli.drop_retention_policy('somename', 'db') + # recreate the RP + self.cli.create_retention_policy('somename', '1w', 1, + shard_duration='1h') + + rsp = self.cli.get_list_retention_policies() + self.assertEqual( + [ + {'duration': '0s', + 'default': True, + 'replicaN': 1, + 'shardGroupDuration': u'168h0m0s', + 'name': 'autogen'}, + {'duration': '168h0m0s', + 'default': False, + 'replicaN': 1, + 'shardGroupDuration': u'1h0m0s', + 'name': 'somename'} + ], + rsp + ) + + self.cli.drop_retention_policy('somename', 'db') + # recreate the RP + self.cli.create_retention_policy('somename', '1w', 1) + + rsp = self.cli.get_list_retention_policies() + self.assertEqual( + [ + {'duration': '0s', + 'default': True, + 'replicaN': 1, + 'shardGroupDuration': u'168h0m0s', + 'name': 'autogen'}, + {'duration': '168h0m0s', + 'default': False, + 'replicaN': 1, + 'shardGroupDuration': u'24h0m0s', + 'name': 'somename'} + ], + rsp + ) + def test_alter_retention_policy(self): """Test alter a retention policy, not default.""" self.cli.create_retention_policy('somename', '1d', 1) # Test alter duration self.cli.alter_retention_policy('somename', 'db', - duration='4d') + duration='4d', + shard_duration='2h') # NB: altering retention policy doesn't change shard group duration rsp = self.cli.get_list_retention_policies() self.assertEqual( @@ -545,7 +589,7 @@ def test_alter_retention_policy(self): {'duration': '96h0m0s', 'default': False, 'replicaN': 1, - 'shardGroupDuration': u'1h0m0s', + 'shardGroupDuration': u'2h0m0s', 'name': 'somename'} ], rsp @@ -554,6 +598,7 @@ def test_alter_retention_policy(self): # Test alter replication self.cli.alter_retention_policy('somename', 'db', replication=4) + # NB: altering retention policy doesn't change shard group duration rsp = self.cli.get_list_retention_policies() self.assertEqual( @@ -566,7 +611,7 @@ def test_alter_retention_policy(self): {'duration': '96h0m0s', 'default': False, 'replicaN': 4, - 'shardGroupDuration': u'1h0m0s', + 'shardGroupDuration': u'2h0m0s', 'name': 'somename'} ], rsp @@ -587,7 +632,28 @@ def test_alter_retention_policy(self): {'duration': '96h0m0s', 'default': True, 'replicaN': 4, - 'shardGroupDuration': u'1h0m0s', + 'shardGroupDuration': u'2h0m0s', + 'name': 'somename'} + ], + rsp + ) + + # Test alter shard_duration + self.cli.alter_retention_policy('somename', 'db', + shard_duration='4h') + + rsp = self.cli.get_list_retention_policies() + self.assertEqual( + [ + {'duration': '0s', + 'default': False, + 'replicaN': 1, + 'shardGroupDuration': u'168h0m0s', + 'name': 'autogen'}, + {'duration': '96h0m0s', + 'default': True, + 'replicaN': 4, + 'shardGroupDuration': u'4h0m0s', 'name': 'somename'} ], rsp @@ -598,6 +664,7 @@ def test_alter_retention_policy_invalid(self): self.cli.create_retention_policy('somename', '1d', 1) with self.assertRaises(InfluxDBClientError) as ctx: self.cli.alter_retention_policy('somename', 'db') + print(self.cli.get_list_retention_policies()) self.assertEqual(400, ctx.exception.code) self.assertIn('{"error":"error parsing query: ', ctx.exception.content) From 8e0fc8c1f5c3b24bb606b0315493f4f2ed18af24 Mon Sep 17 00:00:00 2001 From: Matthew McGinn Date: Wed, 4 Jul 2018 00:36:28 -0400 Subject: [PATCH 2/2] Remove debug print statement --- influxdb/tests/server_tests/client_test_with_server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/influxdb/tests/server_tests/client_test_with_server.py b/influxdb/tests/server_tests/client_test_with_server.py index c18aa328..1da1c2e4 100644 --- a/influxdb/tests/server_tests/client_test_with_server.py +++ b/influxdb/tests/server_tests/client_test_with_server.py @@ -664,7 +664,6 @@ def test_alter_retention_policy_invalid(self): self.cli.create_retention_policy('somename', '1d', 1) with self.assertRaises(InfluxDBClientError) as ctx: self.cli.alter_retention_policy('somename', 'db') - print(self.cli.get_list_retention_policies()) self.assertEqual(400, ctx.exception.code) self.assertIn('{"error":"error parsing query: ', ctx.exception.content)