Drop retention policy before creating it #2051

mkpankov · 2020-07-07T11:35:05Z

Otherwise it fails to setup Influx if it's already setup

Signed-off-by: Michael Pankov work@michaelpankov.com

This change is

Signed-off-by: Michael Pankov <work@michaelpankov.com>

ip1981 · 2020-07-07T11:46:54Z

Why don't we need the same for CREATE DATABASE?

ip1981 · 2020-07-07T11:48:43Z

Looks like here begins a discrepancy with docker :)

When changing any of the following also change: docker/influxdb/setup-influxdb.sh

Signed-off-by: Michael Pankov <work@michaelpankov.com>

mkpankov · 2020-07-07T11:52:47Z

@ip1981 fixed the docker.

We don't need it for CREATE DATABASE because it doesn't fail when DB already exists, apparently.

ip1981 · 2020-07-07T12:12:20Z

I don't see it failing. What do I do wrong?

[root@adm ~]# influx -execute 'CREATE RETENTION POLICY "long_term" ON "iml_stats" DURATION 1w REPLICATION 1 SHARD DURATION 5d'
[root@adm ~]# echo $?
0
[root@adm ~]# influx -execute 'CREATE RETENTION POLICY "long_term" ON "iml_stats" DURATION 1w REPLICATION 1 SHARD DURATION 5d'
[root@adm ~]# echo $?
0

mkpankov · 2020-07-07T12:19:40Z

I don't know, for me it looked like this

    adm: ++ chroma-config setup admin lustre localhost --no-dbspace-check
    adm: Starting setup...
    adm: Clearing all sessions...
    adm: Starting InfluxDB...
    adm: Creating InfluxDB database...
    adm: Traceback (most recent call last):
    adm:   File "/bin/chroma-config", line 10, in <module>
    adm:     
    adm: load_entry_point('iml-manager==6.1.0', 'console_scripts', 'chroma-config')()
    adm:   File "/usr/share/chroma-manager/chroma_core/lib/service_config.py", line 1029, in chroma_config
    adm:     errors = service_config.setup(username, password, ntpserver, check_db_space)
    adm:   File "/usr/share/chroma-manager/chroma_core/lib/service_config.py", line 767, in setup
    adm:     self._setup_influxdb()
    adm:   File "/usr/share/chroma-manager/chroma_core/lib/service_config.py", line 347, in _setup_influxdb
    adm:     settings.INFLUXDB_IML_STATS_DB, settings.INFLUXDB_IML_STATS_LONG_DURATION,
    adm:   File "/usr/share/chroma-manager/chroma_core/lib/util.py", line 192, in try_shell
    adm:     raise CommandError(cmdline, rc, out, err)
    adm: chroma_core.lib.util.CommandError
    adm: : Command failed: ['influx', '-database', 'iml_stats', '-execute', 'CREATE RETENTION POLICY "long_term" ON "iml_stats" DURATION 52w REPLICATION 1 SHARD DURATION 5d']
    adm:     return code 1
    adm:     stdout: ERR: retention policy already exists
    adm: 
    adm:     stderr: retention policy already exists
The SSH command responded with a non-zero exit status. Vagrant
assumes that this means the command failed. The output for this command
should be in the log above. Please read the output to determine what
went wrong.

ip1981 · 2020-07-07T12:27:33Z

Oh, I get it. Somehow DURATION is involved.

ip1981 · 2020-07-07T12:29:35Z

So, you can't create a policy with different duration.

ip1981 · 2020-07-07T12:40:13Z

One concern remains:

Dropping a retention policy will permanently delete all measurements and data stored in the retention policy.

It looks like the data might be lost accidentally.

mkpankov · 2020-07-07T12:44:36Z

Isn't it the same with CONTINUOUS QUERY? We drop them too

ip1981 · 2020-07-07T13:05:02Z

Isn't it the same with CONTINUOUS QUERY? We drop them too

We drop the queries, but not the data they produce, AFAIK:

Continuous queries (CQ) are InfluxQL queries that run automatically and periodically on realtime data and store query results in a specified measurement.

jgrund

This needs more investigation / clarification.

According to the influx docs (https://docs.influxdata.com/influxdb/v1.8/query_language/manage-database/#create-retention-policies-with-create-retention-policy)

A successful CREATE RETENTION POLICY query returns an empty response. If you attempt to create a retention policy identical to one that already exists, InfluxDB does not return an error. If you attempt to create a retention policy with the same name as an existing retention policy but with differing attributes, InfluxDB returns an error.

ip1981 · 2020-07-07T13:18:09Z

I think we need CREATE or ALTER if already exists.

mkpankov · 2020-07-07T13:18:30Z

@jgrund I think we want to allow the attributes to be different. In my case it's probably because I was testing tunable retention policy locally, but I'd say this is useful overall.
If this drops produced data though it's not really viable.

mkpankov · 2020-07-07T13:20:38Z

Ok I'll try to determine if it's already present, and ALTER in that case.

jgrund · 2020-07-07T13:23:59Z

Ok I'll try to determine if it's already present, and ALTER in that case.

What happens to existing data when a RP is altered?

mkpankov · 2020-07-08T09:16:26Z

Seems existing data is intact after altering RP

> show retention policies
name      duration  shardGroupDuration replicaN default
----      --------  ------------------ -------- -------
autogen   24h0m0s   2h0m0s             1        true
long_term 8736h0m0s 120h0m0s           1        false
> select * from "iml_stats"."long_term"."host"
name: host
time                host       mean_lnet_mem_used
----                ----       ------------------
1594121400000000000 mds1.local 1765505
1594121400000000000 mds2.local 1765505
1594121400000000000 oss1.local 1765505
1594121400000000000 oss2.local 1765505
1594123200000000000 mds1.local 1765505
1594123200000000000 mds2.local 1765505
1594123200000000000 oss1.local 1765505
1594123200000000000 oss2.local 1765505
1594125000000000000 mds1.local 1765505
1594125000000000000 mds2.local 1765505
1594125000000000000 oss1.local 1765505
1594125000000000000 oss2.local 1765505
1594126800000000000 mds1.local 1765505
1594126800000000000 mds2.local 1765505
1594126800000000000 oss1.local 1765505
1594126800000000000 oss2.local 1765505
1594195200000000000 mds1.local 1764875.2564102565
1594195200000000000 mds2.local 1765505
1594195200000000000 oss1.local 1765505
1594195200000000000 oss2.local 1765505
1594197000000000000 mds1.local 1765505
1594197000000000000 mds2.local 1765505
1594197000000000000 oss1.local 1765505
1594197000000000000 oss2.local 1765505
> alter retention policy long_term on iml_stats duration 1w
> show retention policies
name      duration shardGroupDuration replicaN default
----      -------- ------------------ -------- -------
autogen   24h0m0s  2h0m0s             1        true
long_term 168h0m0s 120h0m0s           1        false
> select * from "iml_stats"."long_term"."host"
name: host
time                host       mean_lnet_mem_used
----                ----       ------------------
1594121400000000000 mds1.local 1765505
1594121400000000000 mds2.local 1765505
1594121400000000000 oss1.local 1765505
1594121400000000000 oss2.local 1765505
1594123200000000000 mds1.local 1765505
1594123200000000000 mds2.local 1765505
1594123200000000000 oss1.local 1765505
1594123200000000000 oss2.local 1765505
1594125000000000000 mds1.local 1765505
1594125000000000000 mds2.local 1765505
1594125000000000000 oss1.local 1765505
1594125000000000000 oss2.local 1765505
1594126800000000000 mds1.local 1765505
1594126800000000000 mds2.local 1765505
1594126800000000000 oss1.local 1765505
1594126800000000000 oss2.local 1765505
1594195200000000000 mds1.local 1764875.2564102565
1594195200000000000 mds2.local 1765505
1594195200000000000 oss1.local 1765505
1594195200000000000 oss2.local 1765505
1594197000000000000 mds1.local 1765505
1594197000000000000 mds2.local 1765505
1594197000000000000 oss1.local 1765505
1594197000000000000 oss2.local 1765505

ip1981 · 2020-07-08T09:30:05Z

I guess that's the point why ALTER exist. Meanwhile continuos queries cannot be altered, only dropped.

jgrund · 2020-07-08T12:21:34Z

Ok, so we should move this patch to use alter on create failure due to existing RP then

Signed-off-by: Michael Pankov <work@michaelpankov.com>

mkpankov · 2020-07-10T06:34:00Z

ALTER on CREATE failure implemented

chroma_core/lib/service_config.py

docker/influxdb/setup-influxdb.sh

chroma_core/lib/service_config.py

Signed-off-by: Michael Pankov <work@michaelpankov.com>

* Drop retention policy before creating it Signed-off-by: Michael Pankov <work@michaelpankov.com> * Do the same for docker Signed-off-by: Michael Pankov <work@michaelpankov.com> * Alter on error Signed-off-by: Michael Pankov <work@michaelpankov.com> * Remove extra format Signed-off-by: Michael Pankov <work@michaelpankov.com> * Don't compare error message Signed-off-by: Michael Pankov <work@michaelpankov.com> * Fix second script Signed-off-by: Michael Pankov <work@michaelpankov.com>

Drop retention policy before creating it

133760d

Signed-off-by: Michael Pankov <work@michaelpankov.com>

mkpankov added the bug label Jul 7, 2020

mkpankov added this to the IML EX V3 milestone Jul 7, 2020

mkpankov requested review from jgrund, utopiabound and a team July 7, 2020 11:35

mkpankov self-assigned this Jul 7, 2020

Do the same for docker

ef33131

Signed-off-by: Michael Pankov <work@michaelpankov.com>

jgrund suggested changes Jul 7, 2020

View reviewed changes

mkpankov added 2 commits July 10, 2020 09:27

Alter on error

8617922

Signed-off-by: Michael Pankov <work@michaelpankov.com>

Remove extra format

103b6ab

Signed-off-by: Michael Pankov <work@michaelpankov.com>

mkpankov requested a review from jgrund July 10, 2020 06:33

jgrund reviewed Jul 10, 2020

View reviewed changes

chroma_core/lib/service_config.py Outdated Show resolved Hide resolved

Merge branch 'master' into drop

a9f67d4

ip1981 reviewed Jul 13, 2020

View reviewed changes

docker/influxdb/setup-influxdb.sh Outdated Show resolved Hide resolved

chroma_core/lib/service_config.py Outdated Show resolved Hide resolved

Don't compare error message

7398bcd

Signed-off-by: Michael Pankov <work@michaelpankov.com>

mkpankov requested a review from jgrund July 14, 2020 07:31

Fix second script

c2329ca

Signed-off-by: Michael Pankov <work@michaelpankov.com>

mkpankov requested a review from a team July 14, 2020 10:59

ip1981 approved these changes Jul 14, 2020

View reviewed changes

jgrund approved these changes Jul 14, 2020

View reviewed changes

jgrund merged commit f5f980a into master Jul 14, 2020

jgrund deleted the drop branch July 14, 2020 13:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop retention policy before creating it #2051

Drop retention policy before creating it #2051

mkpankov commented Jul 7, 2020 •

edited by jgrund

ip1981 commented Jul 7, 2020

ip1981 commented Jul 7, 2020 •

edited

mkpankov commented Jul 7, 2020

ip1981 commented Jul 7, 2020

mkpankov commented Jul 7, 2020

ip1981 commented Jul 7, 2020

ip1981 commented Jul 7, 2020

ip1981 commented Jul 7, 2020

mkpankov commented Jul 7, 2020

ip1981 commented Jul 7, 2020 •

edited

jgrund left a comment

ip1981 commented Jul 7, 2020

mkpankov commented Jul 7, 2020

mkpankov commented Jul 7, 2020 •

edited

jgrund commented Jul 7, 2020

mkpankov commented Jul 8, 2020

ip1981 commented Jul 8, 2020

jgrund commented Jul 8, 2020

mkpankov commented Jul 10, 2020

Drop retention policy before creating it #2051

Drop retention policy before creating it #2051

Conversation

mkpankov commented Jul 7, 2020 • edited by jgrund

ip1981 commented Jul 7, 2020

ip1981 commented Jul 7, 2020 • edited

mkpankov commented Jul 7, 2020

ip1981 commented Jul 7, 2020

mkpankov commented Jul 7, 2020

ip1981 commented Jul 7, 2020

ip1981 commented Jul 7, 2020

ip1981 commented Jul 7, 2020

mkpankov commented Jul 7, 2020

ip1981 commented Jul 7, 2020 • edited

jgrund left a comment

Choose a reason for hiding this comment

ip1981 commented Jul 7, 2020

mkpankov commented Jul 7, 2020

mkpankov commented Jul 7, 2020 • edited

jgrund commented Jul 7, 2020

mkpankov commented Jul 8, 2020

ip1981 commented Jul 8, 2020

jgrund commented Jul 8, 2020

mkpankov commented Jul 10, 2020

mkpankov commented Jul 7, 2020 •

edited by jgrund

ip1981 commented Jul 7, 2020 •

edited

ip1981 commented Jul 7, 2020 •

edited

mkpankov commented Jul 7, 2020 •

edited