From 9ec47a28a3a53a180cfe20237ad005f97be888b6 Mon Sep 17 00:00:00 2001 From: Nico Vinzens Date: Thu, 6 Dec 2018 10:58:58 +0100 Subject: [PATCH 1/6] quote cq names --- influxdb-schema-updater | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/influxdb-schema-updater b/influxdb-schema-updater index 8d86705..1edaf02 100755 --- a/influxdb-schema-updater +++ b/influxdb-schema-updater @@ -146,7 +146,7 @@ sub extract_database_updates { my ($db_schemas_in_influxdb, $db_schemas_in_config, $dryrun, $force) = @_; my ($old_dbs, $eq_dbs, $new_dbs) = get_Ldifference_intersection_Rdifference([keys %{$db_schemas_in_influxdb}], [keys %{$db_schemas_in_config}]); - + my %rp_updates; for my $db (@$eq_dbs, @$new_dbs) { my ($old, $updated, $new) = extract_retention_policy_updates($db, $db_schemas_in_influxdb->{$db}, $db_schemas_in_config->{$db}->{rps}, $dryrun, $force); @@ -276,7 +276,7 @@ sub extract_continuous_query_updates { object => 'cq', db => $db, name => $cq, - query => "DROP CONTINUOUS QUERY $cq ON $db;", + query => qq{DROP CONTINUOUS QUERY "$cq" ON "$db";}, skip => $dryrun || !$force, }; } @@ -287,7 +287,7 @@ sub extract_continuous_query_updates { object => 'cq', db => $db, name => $cq, - query => "DROP CONTINUOUS QUERY $cq ON $db; $all_cqs_in_config->{$db}->{$cq};", + query => qq{DROP CONTINUOUS QUERY "$cq" ON "$db"; $all_cqs_in_config->{$db}->{$cq};}, skip => $dryrun, }; } @@ -392,11 +392,11 @@ sub load_db_schemas_in_config { # this will be populated with all statements parsed from the files my %parsed_statements; my $db_files = get_schema_files_for_dir("$schema_dir/db"); - + for my $db_file (@$db_files) { my $statements_in_file = read_text("$schema_dir/db/$db_file"); my $parsed_statements_from_file = parse_statements($statements_in_file); - + # add the statements from this file to the initial hash %parsed_statements = (%parsed_statements, %$parsed_statements_from_file); } @@ -453,7 +453,7 @@ sub parse_statements { for my $line (@splitted_lines) { # ignore empty and commented lines next if $line =~ /^\s*(#.*)?$/; - + if ($line =~ /$integrated_regex/) { # capture groups from the regex my $create_db_statement = $1; @@ -507,13 +507,13 @@ sub parse_statements { # # Arguments: # $parsed_statements reference: a reference to the hash containing the db statements -# $db_name string: +# $db_name string: # Returns: # - # sub check_if_db_statement_exists_and_die { my ($parsed_statements, $db_name) = @_; - + # there should be no 'create database' statement yet for this database if ($parsed_statements->{$db_name}{create_query}) { die "Create statement already defined for database $db_name. Fix the config file"; From 659cde67ee41c53d073ca0692145eb255ca619de Mon Sep 17 00:00:00 2001 From: Nico Vinzens Date: Thu, 6 Dec 2018 13:50:36 +0100 Subject: [PATCH 2/6] fix tests so they accept double quoted strings --- t/influxdb-schema-updater.t | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/t/influxdb-schema-updater.t b/t/influxdb-schema-updater.t index 8701528..833fa44 100755 --- a/t/influxdb-schema-updater.t +++ b/t/influxdb-schema-updater.t @@ -65,7 +65,7 @@ sub test { is run_updater($curdir, "$schemas_dir/test05", $port, 0, '--diff'), '' => 'CQs are added'; # change a continuous query - is run_updater($curdir, "$schemas_dir/test06", $port, 0, '--diff'), "DROP CONTINUOUS QUERY cq2 ON test; CREATE CONTINUOUS QUERY cq2 ON test RESAMPLE EVERY 5m FOR 10m BEGIN SELECT MAX(a) AS b, c INTO test.rp2.m FROM test.rp1.m GROUP BY time(5m) END;\n" + is run_updater($curdir, "$schemas_dir/test06", $port, 0, '--diff'), qq{DROP CONTINUOUS QUERY "cq2" ON test; CREATE CONTINUOUS QUERY cq2 ON test RESAMPLE EVERY 5m FOR 10m BEGIN SELECT MAX(a) AS b, c INTO test.rp2.m FROM test.rp1.m GROUP BY time(5m) END;\n} => 'CQ change is detected'; run_updater($curdir, "$schemas_dir/test06", $port, 0); is run_updater($curdir, "$schemas_dir/test06", $port, 0, '--diff'), '' => 'CQ is updated'; @@ -76,43 +76,50 @@ sub test { run_updater($curdir, "$schemas_dir/test06", $port, 0, '--force'); # reset # remove a continuous query - is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), "-- DROP CONTINUOUS QUERY cq2 ON test;\n" + is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq2" ON test;\n} => 'CQ removal is detected'; run_updater($curdir, "$schemas_dir/test07", $port, 1); - is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), "-- DROP CONTINUOUS QUERY cq2 ON test;\n" + is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq2" ON test;\n} => 'CQ is not deleted without --force'; # don't execute a delete action be default - return exit code 1 when some changes are not applied is run_updater($curdir, "$schemas_dir/test07", $port, 1), "[!] skipped: delete continuous query cq2 on database test\n" => "Don't execute delete statements without --force"; - + run_updater($curdir, "$schemas_dir/test07", $port, 0, '--force'); is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), '' => 'CQ is deleted with --force'; # test the order of updates - is run_updater($curdir, "$schemas_dir/test08", $port, 0, '--diff', '--force'), "DROP CONTINUOUS QUERY cq1 ON test;\nDROP DATABASE test;\nCREATE DATABASE test2;\nCREATE RETENTION POLICY \"rp1\" ON test2 DURATION 100d REPLICATION 1 SHARD DURATION 2w;\nCREATE RETENTION POLICY \"rp2\" ON test2 DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\nCREATE CONTINUOUS QUERY cq1 ON test2 RESAMPLE EVERY 5m FOR 10m BEGIN SELECT LAST(a) AS b, c INTO test2.rp2.m FROM test2.rp1.m GROUP BY time(5m) END;\n" + is run_updater($curdir, "$schemas_dir/test08", $port, 0, '--diff', '--force'), qq{DROP CONTINUOUS QUERY "cq1" ON test;\nDROP DATABASE test;\nCREATE DATABASE test2;\nCREATE RETENTION POLICY \"rp1\" ON test2 DURATION 100d REPLICATION 1 SHARD DURATION 2w;\nCREATE RETENTION POLICY \"rp2\" ON test2 DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\nCREATE CONTINUOUS QUERY cq1 ON test2 RESAMPLE EVERY 5m FOR 10m BEGIN SELECT LAST(a) AS b, c INTO test2.rp2.m FROM test2.rp1.m GROUP BY time(5m) END;\n} => 'Updates applied in the right order'; - is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), "-- DROP CONTINUOUS QUERY cq1 ON test;\n-- DROP DATABASE test;\n" + is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON test;\n-- DROP DATABASE test;\n} => 'Old database is detected'; run_updater($curdir, "$schemas_dir/test00", $port, 1); - is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), "-- DROP CONTINUOUS QUERY cq1 ON test;\n-- DROP DATABASE test;\n" + is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON test;\n-- DROP DATABASE test;\n} => 'Database is not deleted without --force'; - + run_updater($curdir, "$schemas_dir/test00", $port, 0, '--force'); is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), '' => 'Database is deleted with --force'; - + # Exit with error when a database is created a second time run_updater($curdir, "$schemas_dir/test10", $port, 255, '--diff'); - + ($pid, $tmpdir_handle) = restart_db($pid); is run_updater($curdir, "$schemas_dir/test12", $port, 0, '--diff'), '' => 'Comments are ignored'; - + is run_updater($curdir, "$schemas_dir/test13", $port, 0, '--diff'), "CREATE DATABASE test1;\nCREATE DATABASE test2;\nCREATE DATABASE test3;\n" => 'Multiple config files are handled properly'; - + run_updater($curdir, "$schemas_dir/test02", $port, 0); is run_updater($curdir, "$schemas_dir/test02", $port, 0, '--diff'), '' => 'Running the updater a second time for the same config does nothing (regression LAKE-338)'; - + + # Test for handling of name with dots (to test a PR) + #($pid, $tmpdir_handle) = restart_db($pid); + #is run_updater($curdir, "$schemas_dir/test_names_with_dot", $port, 0, '--diff'), "DROP CONTINUOUS QUERY cq2 ON test; CREATE CONTINUOUS QUERY cq2 ON test RESAMPLE EVERY 5m FOR 10m BEGIN SELECT MAX(a) AS b, c INTO test.rp2.m FROM test.rp1.m GROUP BY time(5m) END;\n" + # => 'CQ change is detected'; + #run_updater($curdir, "$schemas_dir/test_names_with_dot", $port, 0); + #is run_updater($curdir, "$schemas_dir/test_names_with_dot", $port, 0, '--diff'), '' => 'CQ is updated'; + done_testing(); @@ -156,7 +163,7 @@ sub start_db { # # Arguments: # $old_pid int: the pid of the currently running InfluxDB -# +# # Returns: # $pid int: the pid of the new InfluxDB # $tmpdir_handle: the file handler for the Influx config directory. We return it so that the file has always a reference to it, otherwise GC might delete it. @@ -188,7 +195,7 @@ sub run_updater { my @cmd = ("$curdir/../influxdb-schema-updater", '--config', $schema_dir, '--url', "localhost:$port", @flags); my $output = run_cmd(@cmd); is $? >> 8, $exit_code, "expected exit code for @cmd"; - + return $output } From c04ab9241d208a8c31d489c82aa8a243d5e8899d Mon Sep 17 00:00:00 2001 From: Nico Vinzens Date: Thu, 6 Dec 2018 13:56:37 +0100 Subject: [PATCH 3/6] fix tests #2 --- t/influxdb-schema-updater.t | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/t/influxdb-schema-updater.t b/t/influxdb-schema-updater.t index 833fa44..6008fd0 100755 --- a/t/influxdb-schema-updater.t +++ b/t/influxdb-schema-updater.t @@ -65,7 +65,7 @@ sub test { is run_updater($curdir, "$schemas_dir/test05", $port, 0, '--diff'), '' => 'CQs are added'; # change a continuous query - is run_updater($curdir, "$schemas_dir/test06", $port, 0, '--diff'), qq{DROP CONTINUOUS QUERY "cq2" ON test; CREATE CONTINUOUS QUERY cq2 ON test RESAMPLE EVERY 5m FOR 10m BEGIN SELECT MAX(a) AS b, c INTO test.rp2.m FROM test.rp1.m GROUP BY time(5m) END;\n} + is run_updater($curdir, "$schemas_dir/test06", $port, 0, '--diff'), qq{DROP CONTINUOUS QUERY "cq2" ON "test"; CREATE CONTINUOUS QUERY cq2 ON test RESAMPLE EVERY 5m FOR 10m BEGIN SELECT MAX(a) AS b, c INTO test.rp2.m FROM test.rp1.m GROUP BY time(5m) END;\n} => 'CQ change is detected'; run_updater($curdir, "$schemas_dir/test06", $port, 0); is run_updater($curdir, "$schemas_dir/test06", $port, 0, '--diff'), '' => 'CQ is updated'; @@ -76,10 +76,10 @@ sub test { run_updater($curdir, "$schemas_dir/test06", $port, 0, '--force'); # reset # remove a continuous query - is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq2" ON test;\n} + is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq2" ON "test";\n} => 'CQ removal is detected'; run_updater($curdir, "$schemas_dir/test07", $port, 1); - is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq2" ON test;\n} + is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq2" ON "test";\n} => 'CQ is not deleted without --force'; # don't execute a delete action be default - return exit code 1 when some changes are not applied is run_updater($curdir, "$schemas_dir/test07", $port, 1), "[!] skipped: delete continuous query cq2 on database test\n" => "Don't execute delete statements without --force"; @@ -88,14 +88,14 @@ sub test { is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), '' => 'CQ is deleted with --force'; # test the order of updates - is run_updater($curdir, "$schemas_dir/test08", $port, 0, '--diff', '--force'), qq{DROP CONTINUOUS QUERY "cq1" ON test;\nDROP DATABASE test;\nCREATE DATABASE test2;\nCREATE RETENTION POLICY \"rp1\" ON test2 DURATION 100d REPLICATION 1 SHARD DURATION 2w;\nCREATE RETENTION POLICY \"rp2\" ON test2 DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\nCREATE CONTINUOUS QUERY cq1 ON test2 RESAMPLE EVERY 5m FOR 10m BEGIN SELECT LAST(a) AS b, c INTO test2.rp2.m FROM test2.rp1.m GROUP BY time(5m) END;\n} + is run_updater($curdir, "$schemas_dir/test08", $port, 0, '--diff', '--force'), qq{DROP CONTINUOUS QUERY "cq1" ON "test";\nDROP DATABASE test;\nCREATE DATABASE test2;\nCREATE RETENTION POLICY \"rp1\" ON test2 DURATION 100d REPLICATION 1 SHARD DURATION 2w;\nCREATE RETENTION POLICY \"rp2\" ON test2 DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\nCREATE CONTINUOUS QUERY cq1 ON test2 RESAMPLE EVERY 5m FOR 10m BEGIN SELECT LAST(a) AS b, c INTO test2.rp2.m FROM test2.rp1.m GROUP BY time(5m) END;\n} => 'Updates applied in the right order'; - is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON test;\n-- DROP DATABASE test;\n} + is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON "test";\n-- DROP DATABASE test;\n} => 'Old database is detected'; run_updater($curdir, "$schemas_dir/test00", $port, 1); - is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON test;\n-- DROP DATABASE test;\n} + is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON "test";\n-- DROP DATABASE test;\n} => 'Database is not deleted without --force'; run_updater($curdir, "$schemas_dir/test00", $port, 0, '--force'); From e6e1132997ff1e9774fc0741fae8a88fe108df02 Mon Sep 17 00:00:00 2001 From: Nico Vinzens Date: Mon, 10 Dec 2018 12:04:08 +0100 Subject: [PATCH 4/6] refactor handling of db and rp creation, so everything can be handled the same, TODO: cleanup the code --- influxdb-schema-updater | 87 +++++++++++++++++++++++++------------ t/influxdb-schema-updater.t | 30 ++++++------- 2 files changed, 74 insertions(+), 43 deletions(-) diff --git a/influxdb-schema-updater b/influxdb-schema-updater index 1edaf02..75471da 100755 --- a/influxdb-schema-updater +++ b/influxdb-schema-updater @@ -149,7 +149,7 @@ sub extract_database_updates { my %rp_updates; for my $db (@$eq_dbs, @$new_dbs) { - my ($old, $updated, $new) = extract_retention_policy_updates($db, $db_schemas_in_influxdb->{$db}, $db_schemas_in_config->{$db}->{rps}, $dryrun, $force); + my ($old, $updated, $new) = extract_retention_policy_updates($db, $db_schemas_in_influxdb->{$db}, $db_schemas_in_config->{$db}, $dryrun, $force); $rp_updates{old_rps}->{$db} = $old; $rp_updates{updated_rps}->{$db} = $updated; $rp_updates{new_rps}->{$db} = $new; @@ -169,7 +169,7 @@ sub extract_database_updates { object => 'db', db => $db, name => $db, - query => "DROP DATABASE $db;", + query => qq{DROP DATABASE "$db";}, skip => $dryrun || !$force, }; } @@ -180,7 +180,7 @@ sub extract_database_updates { object => 'db', db => $db, name => $db, - query => $db_schemas_in_config->{$db}->{create_query}, + query => qq{CREATE DATABASE "$db";}, skip => $dryrun, }; } @@ -209,7 +209,7 @@ sub extract_retention_policy_updates { object => 'rp', db => $db, name => $rp, - query => "DROP RETENTION POLICY \"$rp\" ON $db;", + query => qq{DROP RETENTION POLICY "$rp" ON "$db";}, skip => $dryrun || !$force, }; } @@ -221,7 +221,7 @@ sub extract_retention_policy_updates { object => 'rp', db => $db, name => $rp, - query => "ALTER RETENTION POLICY \"$rp\" ON $db DURATION $rps_in_config->{$rp}->{duration} REPLICATION 1 SHARD DURATION $rps_in_config->{$rp}->{shard_duration}" . ($rps_in_config->{$rp}->{default} ? ' DEFAULT;' : ';'), + query => qq{ALTER RETENTION POLICY "$rp" ON "$db" DURATION $rps_in_config->{$rp}->{duration} REPLICATION 1 SHARD DURATION $rps_in_config->{$rp}->{shard_duration}} . ($rps_in_config->{$rp}->{default} ? ' DEFAULT;' : ';'), skip => $dryrun, }; } @@ -233,7 +233,7 @@ sub extract_retention_policy_updates { object => 'rp', db => $db, name => $rp, - query => "CREATE RETENTION POLICY \"$rp\" ON $db DURATION $rps_in_config->{$rp}->{duration} REPLICATION $rps_in_config->{$rp}->{rp_replication} SHARD DURATION $rps_in_config->{$rp}->{shard_duration}" . ($rps_in_config->{$rp}->{default} ? ' DEFAULT;' : ';'), + query => qq{CREATE RETENTION POLICY "$rp" ON "$db" DURATION $rps_in_config->{$rp}->{duration} REPLICATION $rps_in_config->{$rp}->{rp_replication} SHARD DURATION $rps_in_config->{$rp}->{shard_duration}} . ($rps_in_config->{$rp}->{default} ? ' DEFAULT;' : ';'), skip => $dryrun, }; } @@ -346,7 +346,7 @@ sub load_db_schemas_in_influxdb { my %db_schemas_in_influxdb; for my $db (@dbs_in_influxdb) { - my $rp_query_res = query_influxql($influxdb_client, "SHOW RETENTION POLICIES ON $db"); + my $rp_query_res = query_influxql($influxdb_client, qq{SHOW RETENTION POLICIES ON "$db"}); $db_schemas_in_influxdb{$db} = { map { $_->[0] => { duration => $_->[1], @@ -370,12 +370,10 @@ sub load_db_schemas_in_influxdb { # # Returns: # a reference to a hash holding the parsed statements. This hash is structured as below. -# +# TODO: fix indentation # { -# => { -# create_query => '...', -# rps => { -# => { +# => { +# => { # duration => ..., # shard_duration => ..., # default => ..., @@ -395,15 +393,35 @@ sub load_db_schemas_in_config { for my $db_file (@$db_files) { my $statements_in_file = read_text("$schema_dir/db/$db_file"); - my $parsed_statements_from_file = parse_statements($statements_in_file); + #my $parsed_statements_from_file = parse_statements($statements_in_file); + my ($databases, $rps) = parse_statements($statements_in_file); # add the statements from this file to the initial hash - %parsed_statements = (%parsed_statements, %$parsed_statements_from_file); + # TODO: merge database list and rp list to create a hash which only contains each database and each rp policy once + # TODO: hash: database name -> rp name -> + + for my $db (@$databases) { + if (exists $parsed_statements{$db}) { + die "duplicate database $db in file $db_file detected\n"; + } + $parsed_statements{$db} = {}; + } + for my $rp (@$rps) { + my $db = $rp->{database}; + if (exists $parsed_statements{$db}) { + if (exists $parsed_statements{$db}->{$rp->{rp_name}}) { + die "duplicate rp $rp on db $db in file $db_file detected\n"; + } + $parsed_statements{$db}->{$rp->{rp_name}} = $rp; + } + else { + die "database $db specified in rp $rp from file $db_file does not exist\n"; + } + } } return \%parsed_statements; } - # # Iterates over the lines of a config file, parses the valid statements and adds them to the given hash. # Valid statements are: @@ -418,13 +436,15 @@ sub load_db_schemas_in_config { # a reference to a hash holding the parsed statements from the file. # sub parse_statements { + # TODO: should return 2 lists: 1 list of databases and 1 list of retention policies my ($string_to_parse) = @_; # we want to iterate line-by-line my @splitted_lines = split "\n", $string_to_parse; - my %parsed_statements; + my @databases; + my @rps; # captures a 'create database' statement with an optional retention policy definition - my $db_regex = qr/^\s*(create \s+ database \s+ (\w+)) # a create db statement, optionally starting with a space. Any valid word as db name. Captured group. + my $db_regex = qr/^\s*(create \s+ database \s+ "?([\w.]+)"?) # a create db statement, optionally starting with a space. Any valid word as db name. Captured group. ( \s+ with \s+ duration # optional statement to define a retention policy in the db creation statement \s+ ((?:\d+[smhdw])|(?:inf)) # captured policy duration as one or more numbers followed by a letter, or 'inf' @@ -472,23 +492,34 @@ sub parse_statements { # the line is a 'create database' statement... if ($create_db_statement) { - check_if_db_statement_exists_and_die(\%parsed_statements, $db_name); - $parsed_statements{$db_name}{create_query} = $create_db_statement . ';'; + # TODO: remove: check_if_db_statement_exists_and_die(\%parsed_statements, $db_name); + push @databases, $db_name; + # $parsed_statements{$db_name}{create_query} = $create_db_statement . ';'; # ...and has a retention policy defined inline if ($inline_rp_statement) { # flagged as the default policy (assume that the inline policy is the default) - $parsed_statements{$db_name}{rps}{$inline_rp_name} = {duration => $inline_rp_duration, - shard_duration => $inline_rp_shard_duration, - rp_replication => $inline_rp_replication, - default => 1}; + #TODO: hash -> new list + push @rps, { + database => $db_name, + rp_name => $inline_rp_name, + duration => $inline_rp_duration, + shard_duration => $inline_rp_shard_duration, + rp_replication => $inline_rp_replication, + default => 1, + }; } } # the line is a 'create retention policy' statement + # TODO: add to rp list elsif ($rp_name) { - $parsed_statements{$rp_db_name}{rps}{$rp_name} = {duration => $rp_duration, - shard_duration => $rp_shard_duration, - rp_replication => $rp_replication, - default => $default_rp} + push @rps, { + database => $rp_db_name, + rp_name => $rp_name, + duration => $rp_duration, + shard_duration => $rp_shard_duration, + rp_replication => $rp_replication, + default => $default_rp, + } } else { die "error, should never reach this" @@ -498,7 +529,7 @@ sub parse_statements { die "could not parse input: $line"; } } - return \%parsed_statements; + return return \@databases, \@rps; } diff --git a/t/influxdb-schema-updater.t b/t/influxdb-schema-updater.t index 6008fd0..fdc5a35 100755 --- a/t/influxdb-schema-updater.t +++ b/t/influxdb-schema-updater.t @@ -31,27 +31,27 @@ sub test { is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), '' => 'Empty config'; # only database - is run_updater($curdir, "$schemas_dir/test01", $port, 0, '--diff'), "CREATE DATABASE test;\n" + is run_updater($curdir, "$schemas_dir/test01", $port, 0, '--diff'), qq{CREATE DATABASE "test";\n} => 'New database is detected'; - is run_updater($curdir, "$schemas_dir/test01", $port, 0, '--diff'), "CREATE DATABASE test;\n" + is run_updater($curdir, "$schemas_dir/test01", $port, 0, '--diff'), qq{CREATE DATABASE "test";\n} => '--diff mode doesn\'t update InfluxDB'; run_updater($curdir, "$schemas_dir/test01", $port, 0); is run_updater($curdir, "$schemas_dir/test01", $port, 0, '--diff'), '' => 'Database is added'; # add a retention policy - is run_updater($curdir, "$schemas_dir/test02", $port, 0, '--diff'), "CREATE RETENTION POLICY \"rp1\" ON test DURATION 90d REPLICATION 1 SHARD DURATION 2w;\n" + is run_updater($curdir, "$schemas_dir/test02", $port, 0, '--diff'), qq{CREATE RETENTION POLICY "rp1" ON "test" DURATION 90d REPLICATION 1 SHARD DURATION 2w;\n} => 'New RP is detected'; run_updater($curdir, "$schemas_dir/test02", $port, 0); is run_updater($curdir, "$schemas_dir/test02", $port, 0, '--diff'), '' => 'RP is added'; # change a retention policy - is run_updater($curdir, "$schemas_dir/test03", $port, 0, '--diff'), "ALTER RETENTION POLICY \"rp1\" ON test DURATION 100d REPLICATION 1 SHARD DURATION 2w;\n" + is run_updater($curdir, "$schemas_dir/test03", $port, 0, '--diff'), qq{ALTER RETENTION POLICY "rp1" ON "test" DURATION 100d REPLICATION 1 SHARD DURATION 2w;\n} => 'RP change is detected'; run_updater($curdir, "$schemas_dir/test03", $port, 0); is run_updater($curdir, "$schemas_dir/test03", $port, 0, '--diff'), '' => 'RP is updated'; # create a retention policy on the same line as the database - is run_updater($curdir, "$schemas_dir/test04", $port, 0, '--diff'), "CREATE RETENTION POLICY \"rp2\" ON test DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\n" + is run_updater($curdir, "$schemas_dir/test04", $port, 0, '--diff'), qq{CREATE RETENTION POLICY "rp2" ON "test" DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\n} => 'RP on same line as create database is detected'; run_updater($curdir, "$schemas_dir/test04", $port, 0, '--force'); @@ -88,14 +88,14 @@ sub test { is run_updater($curdir, "$schemas_dir/test07", $port, 0, '--diff'), '' => 'CQ is deleted with --force'; # test the order of updates - is run_updater($curdir, "$schemas_dir/test08", $port, 0, '--diff', '--force'), qq{DROP CONTINUOUS QUERY "cq1" ON "test";\nDROP DATABASE test;\nCREATE DATABASE test2;\nCREATE RETENTION POLICY \"rp1\" ON test2 DURATION 100d REPLICATION 1 SHARD DURATION 2w;\nCREATE RETENTION POLICY \"rp2\" ON test2 DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\nCREATE CONTINUOUS QUERY cq1 ON test2 RESAMPLE EVERY 5m FOR 10m BEGIN SELECT LAST(a) AS b, c INTO test2.rp2.m FROM test2.rp1.m GROUP BY time(5m) END;\n} + is run_updater($curdir, "$schemas_dir/test08", $port, 0, '--diff', '--force'), qq{DROP CONTINUOUS QUERY "cq1" ON "test";\nDROP DATABASE "test";\nCREATE DATABASE "test2";\nCREATE RETENTION POLICY "rp1" ON "test2" DURATION 100d REPLICATION 1 SHARD DURATION 2w;\nCREATE RETENTION POLICY "rp2" ON "test2" DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\nCREATE CONTINUOUS QUERY cq1 ON test2 RESAMPLE EVERY 5m FOR 10m BEGIN SELECT LAST(a) AS b, c INTO test2.rp2.m FROM test2.rp1.m GROUP BY time(5m) END;\n} => 'Updates applied in the right order'; - is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON "test";\n-- DROP DATABASE test;\n} + is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON "test";\n-- DROP DATABASE "test";\n} => 'Old database is detected'; run_updater($curdir, "$schemas_dir/test00", $port, 1); - is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON "test";\n-- DROP DATABASE test;\n} + is run_updater($curdir, "$schemas_dir/test00", $port, 0, '--diff'), qq{-- DROP CONTINUOUS QUERY "cq1" ON "test";\n-- DROP DATABASE "test";\n} => 'Database is not deleted without --force'; run_updater($curdir, "$schemas_dir/test00", $port, 0, '--force'); @@ -108,17 +108,17 @@ sub test { ($pid, $tmpdir_handle) = restart_db($pid); is run_updater($curdir, "$schemas_dir/test12", $port, 0, '--diff'), '' => 'Comments are ignored'; - is run_updater($curdir, "$schemas_dir/test13", $port, 0, '--diff'), "CREATE DATABASE test1;\nCREATE DATABASE test2;\nCREATE DATABASE test3;\n" => 'Multiple config files are handled properly'; + is run_updater($curdir, "$schemas_dir/test13", $port, 0, '--diff'), qq{CREATE DATABASE "test1";\nCREATE DATABASE "test2";\nCREATE DATABASE "test3";\n} => 'Multiple config files are handled properly'; run_updater($curdir, "$schemas_dir/test02", $port, 0); is run_updater($curdir, "$schemas_dir/test02", $port, 0, '--diff'), '' => 'Running the updater a second time for the same config does nothing (regression LAKE-338)'; - # Test for handling of name with dots (to test a PR) - #($pid, $tmpdir_handle) = restart_db($pid); - #is run_updater($curdir, "$schemas_dir/test_names_with_dot", $port, 0, '--diff'), "DROP CONTINUOUS QUERY cq2 ON test; CREATE CONTINUOUS QUERY cq2 ON test RESAMPLE EVERY 5m FOR 10m BEGIN SELECT MAX(a) AS b, c INTO test.rp2.m FROM test.rp1.m GROUP BY time(5m) END;\n" - # => 'CQ change is detected'; - #run_updater($curdir, "$schemas_dir/test_names_with_dot", $port, 0); - #is run_updater($curdir, "$schemas_dir/test_names_with_dot", $port, 0, '--diff'), '' => 'CQ is updated'; + # Test for handling of name with dots (bugfix while working on MON-2086) + ($pid, $tmpdir_handle) = restart_db($pid); + is run_updater($curdir, "$schemas_dir/test_name_with_dot", $port, 0, '--diff'), qq{CREATE DATABASE "db.test";\nCREATE RETENTION POLICY "rp.test" ON "db.test" DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\n} + => 'CQ change is detected'; + run_updater($curdir, "$schemas_dir/test_name_with_dot", $port, 0); + is run_updater($curdir, "$schemas_dir/test_name_with_dot", $port, 0, '--diff'), '' => 'CQ is updated'; done_testing(); From 0e0c03da65bc23bb31b540366dece8a713950cd0 Mon Sep 17 00:00:00 2001 From: Nico Vinzens Date: Mon, 10 Dec 2018 12:04:47 +0100 Subject: [PATCH 5/6] add test case for bugfix --- t/data/test_name_with_dot/cq/db1.ifql | 2 ++ t/data/test_name_with_dot/db/db1.ifql | 1 + 2 files changed, 3 insertions(+) create mode 100644 t/data/test_name_with_dot/cq/db1.ifql create mode 100644 t/data/test_name_with_dot/db/db1.ifql diff --git a/t/data/test_name_with_dot/cq/db1.ifql b/t/data/test_name_with_dot/cq/db1.ifql new file mode 100644 index 0000000..79188ff --- /dev/null +++ b/t/data/test_name_with_dot/cq/db1.ifql @@ -0,0 +1,2 @@ +CREATE CONTINUOUS QUERY "cq.test" ON "db.test" RESAMPLE EVERY 5m FOR 10m BEGIN SELECT LAST(a) AS b, c INTO "rp.test"."m.test" FROM "rp.test"."m.test" GROUP BY time(5m) END; + diff --git a/t/data/test_name_with_dot/db/db1.ifql b/t/data/test_name_with_dot/db/db1.ifql new file mode 100644 index 0000000..713d223 --- /dev/null +++ b/t/data/test_name_with_dot/db/db1.ifql @@ -0,0 +1 @@ +CREATE DATABASE "db.test" WITH DURATION 260w REPLICATION 1 SHARD DURATION 12w NAME "rp.test"; From 339356daa0104cfd61617dfc2f825abd7111043e Mon Sep 17 00:00:00 2001 From: Nico Vinzens Date: Mon, 10 Dec 2018 14:17:23 +0100 Subject: [PATCH 6/6] cleanup --- influxdb-schema-updater | 63 +++++++++++-------------------------- t/influxdb-schema-updater.t | 4 +-- 2 files changed, 20 insertions(+), 47 deletions(-) diff --git a/influxdb-schema-updater b/influxdb-schema-updater index 75471da..a9a09d6 100755 --- a/influxdb-schema-updater +++ b/influxdb-schema-updater @@ -369,11 +369,11 @@ sub load_db_schemas_in_influxdb { # $schema_dir string: the directory name where the config files are # # Returns: -# a reference to a hash holding the parsed statements. This hash is structured as below. -# TODO: fix indentation +# $db_schemas: a reference to a hash holding the parsed statements. This hash is structured as below. +# # { -# => { -# => { +# => { +# => { # duration => ..., # shard_duration => ..., # default => ..., @@ -387,39 +387,37 @@ sub load_db_schemas_in_influxdb { sub load_db_schemas_in_config { my ($schema_dir) = @_; - # this will be populated with all statements parsed from the files - my %parsed_statements; + my %db_schemas; my $db_files = get_schema_files_for_dir("$schema_dir/db"); for my $db_file (@$db_files) { my $statements_in_file = read_text("$schema_dir/db/$db_file"); - #my $parsed_statements_from_file = parse_statements($statements_in_file); my ($databases, $rps) = parse_statements($statements_in_file); - # add the statements from this file to the initial hash - # TODO: merge database list and rp list to create a hash which only contains each database and each rp policy once - # TODO: hash: database name -> rp name -> - + # loop all the databases and add them to the hash for my $db (@$databases) { - if (exists $parsed_statements{$db}) { + # make sure every database is only created once + if (exists $db_schemas{$db}) { die "duplicate database $db in file $db_file detected\n"; } - $parsed_statements{$db} = {}; + $db_schemas{$db} = {}; } + # loop all the retention policies and assign them to the correct database for my $rp (@$rps) { my $db = $rp->{database}; - if (exists $parsed_statements{$db}) { - if (exists $parsed_statements{$db}->{$rp->{rp_name}}) { + if (exists $db_schemas{$db}) { + # make sure every retention policy is only created once + if (exists $db_schemas{$db}->{$rp->{rp_name}}) { die "duplicate rp $rp on db $db in file $db_file detected\n"; } - $parsed_statements{$db}->{$rp->{rp_name}} = $rp; + $db_schemas{$db}->{$rp->{rp_name}} = $rp; } else { die "database $db specified in rp $rp from file $db_file does not exist\n"; } } } - return \%parsed_statements; + return \%db_schemas; } # @@ -430,13 +428,12 @@ sub load_db_schemas_in_config { # # Arguments: # $string_to_parse string: the contents of the config file (loaded as string) -# $parsed_statements reference: a reference to a hash. The function will populate it with the parsed statements. # # Returns: -# a reference to a hash holding the parsed statements from the file. +# $databases: reference to a list which contains all the database names +# $rps: reference to a list which contains hashes of all the retention policies # sub parse_statements { - # TODO: should return 2 lists: 1 list of databases and 1 list of retention policies my ($string_to_parse) = @_; # we want to iterate line-by-line my @splitted_lines = split "\n", $string_to_parse; @@ -492,13 +489,10 @@ sub parse_statements { # the line is a 'create database' statement... if ($create_db_statement) { - # TODO: remove: check_if_db_statement_exists_and_die(\%parsed_statements, $db_name); push @databases, $db_name; - # $parsed_statements{$db_name}{create_query} = $create_db_statement . ';'; # ...and has a retention policy defined inline if ($inline_rp_statement) { - # flagged as the default policy (assume that the inline policy is the default) - #TODO: hash -> new list + # flagged as the default policy (assume that the inline policy is the default)ƒ push @rps, { database => $db_name, rp_name => $inline_rp_name, @@ -510,7 +504,6 @@ sub parse_statements { } } # the line is a 'create retention policy' statement - # TODO: add to rp list elsif ($rp_name) { push @rps, { database => $rp_db_name, @@ -532,26 +525,6 @@ sub parse_statements { return return \@databases, \@rps; } - -# -# Checks if the db creation statement is already added in the parsed statements hash. If yes then it throws an exception. -# -# Arguments: -# $parsed_statements reference: a reference to the hash containing the db statements -# $db_name string: -# Returns: -# - -# -sub check_if_db_statement_exists_and_die { - my ($parsed_statements, $db_name) = @_; - - # there should be no 'create database' statement yet for this database - if ($parsed_statements->{$db_name}{create_query}) { - die "Create statement already defined for database $db_name. Fix the config file"; - } -} - - # { # => { # => , diff --git a/t/influxdb-schema-updater.t b/t/influxdb-schema-updater.t index fdc5a35..a8c26a6 100755 --- a/t/influxdb-schema-updater.t +++ b/t/influxdb-schema-updater.t @@ -116,9 +116,9 @@ sub test { # Test for handling of name with dots (bugfix while working on MON-2086) ($pid, $tmpdir_handle) = restart_db($pid); is run_updater($curdir, "$schemas_dir/test_name_with_dot", $port, 0, '--diff'), qq{CREATE DATABASE "db.test";\nCREATE RETENTION POLICY "rp.test" ON "db.test" DURATION 260w REPLICATION 1 SHARD DURATION 12w DEFAULT;\n} - => 'CQ change is detected'; + => 'Detect changes with dot in the names.'; run_updater($curdir, "$schemas_dir/test_name_with_dot", $port, 0); - is run_updater($curdir, "$schemas_dir/test_name_with_dot", $port, 0, '--diff'), '' => 'CQ is updated'; + is run_updater($curdir, "$schemas_dir/test_name_with_dot", $port, 0, '--diff'), '' => 'Applied changes with dot in the names'; done_testing();