Enabled to use only last N samples of each timeserie on compare tool (#338)

filipecosta90 · web-flow · commit b3b2d8c618d6 · 2022-03-31T15:42:27.000+01:00
* Updated ann-benchmark to the latest pkg version and extend tests on create_website.py

* Enabled to use only last N samples of each timeserie on redisbench-admin compare

* Fixed bug introduced in last_n compare tool
diff --git a/pyproject.toml b/pyproject.toml
@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "redisbench-admin"
-version = "0.7.35"
+version = "0.7.40"
 description = "Redis benchmark run helper. A wrapper around Redis and Redis Modules benchmark tools ( ftsb_redisearch, memtier_benchmark, redis-benchmark, aibench, etc... )."
 authors = ["filipecosta90 <filipecosta.90@gmail.com>","Redis Performance Group <performance@redis.com>"]
 readme = "README.md"
diff --git a/redisbench_admin/compare/args.py b/redisbench_admin/compare/args.py
@@ -47,6 +47,12 @@ def create_compare_arguments(parser):
     parser.add_argument("--baseline_deployment_name", type=str, default="")
     parser.add_argument("--comparison_deployment_name", type=str, default="")
     parser.add_argument("--metric_name", type=str, default="Tests.Overall.rps")
+    parser.add_argument(
+        "--last_n",
+        type=int,
+        default=-1,
+        help="Use the last N samples for each time-serie. by default will use all available values",
+    )
     parser.add_argument(
         "--from-date",
         type=lambda s: datetime.datetime.strptime(s, "%Y-%m-%d"),
diff --git a/redisbench_admin/compare/compare.py b/redisbench_admin/compare/compare.py
@@ -54,6 +54,12 @@ def compare_command_logic(args, project_name, project_version):
             comparison_deployment_name,
         )
     )
+    if args.last_n > 0:
+        logging.info(
+            "Using the last {} samples of each timeserie to compute the tables".format(
+                args.last_n
+            )
+        )
     from_ts_ms = args.from_timestamp
     to_ts_ms = args.to_timestamp
     if from_ts_ms is None:
@@ -161,7 +167,7 @@ def compare_command_logic(args, project_name, project_version):
     total_stable = 0
     total_unstable = 0
     total_regressions = 0
-    noise_waterline = 2.5
+    noise_waterline = 3
     progress = tqdm(unit="benchmark time-series", total=len(test_names))
     for test_name in test_names:
         filters_baseline = [
@@ -234,7 +240,10 @@ def compare_command_logic(args, project_name, project_version):
             if baseline_nsamples > 0:
                 _, baseline_v = baseline_datapoints[0]
                 for tuple in baseline_datapoints:
-                    baseline_values.append(tuple[1])
+                    if args.last_n < 0 or (
+                        args.last_n > 0 and len(baseline_values) < args.last_n
+                    ):
+                        baseline_values.append(tuple[1])
                 baseline_df = pd.DataFrame(baseline_values)
                 baseline_median = float(baseline_df.median())
                 baseline_v = baseline_median
@@ -249,7 +258,10 @@ def compare_command_logic(args, project_name, project_version):
             if comparison_nsamples > 0:
                 _, comparison_v = comparison_datapoints[0]
                 for tuple in comparison_datapoints:
-                    comparison_values.append(tuple[1])
+                    if args.last_n < 0 or (
+                        args.last_n > 0 and len(comparison_values) < args.last_n
+                    ):
+                        comparison_values.append(tuple[1])
                 comparison_df = pd.DataFrame(comparison_values)
                 comparison_median = float(comparison_df.median())
                 comparison_v = comparison_median
@@ -275,13 +287,13 @@ def compare_command_logic(args, project_name, project_version):
             if baseline_pct_change > 10.0:
                 stamp_b = "UNSTABLE"
             baseline_v_str = " {:.0f} +- {:.1f}% {} ({} datapoints)".format(
-                baseline_v, baseline_pct_change, stamp_b, baseline_nsamples
+                baseline_v, baseline_pct_change, stamp_b, len(baseline_values)
             )
             stamp_c = ""
             if comparison_pct_change > 10.0:
                 stamp_c = "UNSTABLE"
             comparison_v_str = " {:.0f} +- {:.1f}% {} ({} datapoints)".format(
-                comparison_v, comparison_pct_change, stamp_c, comparison_nsamples
+                comparison_v, comparison_pct_change, stamp_c, len(comparison_values)
             )
             if metric_mode == "higher-better":
                 percentage_change = (
@@ -297,7 +309,7 @@ def compare_command_logic(args, project_name, project_version):
             detected_improvement = False
             if percentage_change < 0.0 and not unstable:
 
-                if percentage_change < -waterline:
+                if -waterline >= percentage_change:
                     detected_regression = True
                     total_regressions = total_regressions + 1
                     note = note + " REGRESSION"
@@ -339,19 +351,22 @@ def compare_command_logic(args, project_name, project_version):
                 )
 
     logging.info("Printing differential analysis between branches")
+
+    baseline = baseline_branch if args.baseline_branch else baseline_tag
+    comparison = comparison_branch if args.comparison_branch else comparison_tag
     writer = MarkdownTableWriter(
         table_name="Comparison between {} and {} for metric: {}. Time Period from {} to {}. (environment used: {})".format(
-            baseline_branch,
-            comparison_branch,
+            baseline,
+            comparison,
             metric_name,
             from_human_str,
             to_human_str,
             baseline_deployment_name,
         ),
         headers=[
             "Test Case",
-            "Baseline {} (median obs. +- std.dev)".format(baseline_branch),
-            "Comparison {} (median obs. +- std.dev)".format(comparison_branch),
+            "Baseline {} (median obs. +- std.dev)".format(baseline),
+            "Comparison {} (median obs. +- std.dev)".format(comparison),
             "% change ({})".format(metric_mode),
             "Note",
         ],
diff --git a/redisbench_admin/environments/oss_cluster.py b/redisbench_admin/environments/oss_cluster.py
@@ -13,6 +13,7 @@
 
 
 def spin_up_local_redis_cluster(
+    binary,
     dbdir,
     shard_count,
     ip,
@@ -27,8 +28,12 @@ def spin_up_local_redis_cluster(
 
     for master_shard_id in range(1, shard_count + 1):
         shard_port = master_shard_id + start_port - 1
-
+        binary = binary
+        if master_shard_id > 1:
+            binary = "redis-server"
+            logging.info("Ignoring redis binary definition for primary shard > 1")
         command, _ = generate_cluster_redis_server_args(
+            binary,
             dbdir,
             local_module_file,
             ip,
@@ -133,6 +138,7 @@ def setup_oss_cluster_from_conns(meet_cmds, redis_conns, shard_count):
 
 
 def generate_cluster_redis_server_args(
+    binary,
     dbdir,
     local_module_file,
     ip,
@@ -145,34 +151,39 @@ def generate_cluster_redis_server_args(
     if logname_prefix is None:
         logname_prefix = ""
     logfile = "{}cluster-node-port-{}.log".format(logname_prefix, port)
+    if type(binary) == list:
+        command = binary
+    else:
+        command = [binary]
     # start redis-server
-    command = [
-        "redis-server",
-        "--appendonly",
-        "no",
-        "--logfile",
-        logfile,
-        "--cluster-enabled",
-        "yes",
-        "--daemonize",
-        daemonize,
-        "--dbfilename",
-        get_cluster_dbfilename(port),
-        "--protected-mode",
-        "no",
-        "--bind",
-        "{}".format(ip),
-        "--cluster-config-file",
-        "cluster-node-port-{}.config".format(port),
-        "--save",
-        "''",
-        "--cluster-announce-ip",
-        "{}".format(ip),
-        "--port",
-        "{}".format(port),
-        "--dir",
-        dbdir,
-    ]
+    command.extend(
+        [
+            "--appendonly",
+            "no",
+            "--logfile",
+            logfile,
+            "--cluster-enabled",
+            "yes",
+            "--daemonize",
+            daemonize,
+            "--dbfilename",
+            get_cluster_dbfilename(port),
+            "--protected-mode",
+            "no",
+            "--bind",
+            "{}".format(ip),
+            "--cluster-config-file",
+            "cluster-node-port-{}.config".format(port),
+            "--save",
+            "''",
+            "--cluster-announce-ip",
+            "{}".format(ip),
+            "--port",
+            "{}".format(port),
+            "--dir",
+            dbdir,
+        ]
+    )
     if configuration_parameters is not None:
         for parameter, parameter_value in configuration_parameters.items():
             command.extend(
diff --git a/redisbench_admin/environments/oss_standalone.py b/redisbench_admin/environments/oss_standalone.py
@@ -50,16 +50,21 @@ def generate_standalone_redis_server_args(
     configuration_parameters=None,
     modules_configuration_parameters_map={},
 ):
+    if type(binary) == list:
+        command = binary
+    else:
+        command = [binary]
     # start redis-server
-    command = [
-        binary,
-        "--save",
-        "",
-        "--port",
-        "{}".format(port),
-        "--dir",
-        dbdir,
-    ]
+    command.extend(
+        [
+            "--save",
+            "",
+            "--port",
+            "{}".format(port),
+            "--dir",
+            dbdir,
+        ]
+    )
     if configuration_parameters is not None:
         for parameter, parameter_value in configuration_parameters.items():
             command.extend(
diff --git a/redisbench_admin/run/cluster.py b/redisbench_admin/run/cluster.py
@@ -122,6 +122,7 @@ def spin_up_redis_cluster_remote_redis(
         shard_port = master_shard_id + start_port - 1
 
         command, logfile = generate_cluster_redis_server_args(
+            "redis-server",
             dbdir_folder,
             remote_module_files,
             server_private_ip,
diff --git a/redisbench_admin/run/common.py b/redisbench_admin/run/common.py
@@ -52,6 +52,7 @@
 )
 
 BENCHMARK_REPETITIONS = int(os.getenv("BENCHMARK_REPETITIONS", 1))
+REDIS_BINARY = os.getenv("REDIS_BINARY", "redis-server")
 
 
 def prepare_benchmark_parameters(
diff --git a/redisbench_admin/run_local/args.py b/redisbench_admin/run_local/args.py
@@ -5,9 +5,11 @@
 #
 
 from redisbench_admin.run.args import common_run_args
+from redisbench_admin.run.common import REDIS_BINARY
 
 
 def create_run_local_arguments(parser):
     parser = common_run_args(parser)
     parser.add_argument("--port", type=int, default=6379)
+    parser.add_argument("--redis-binary", type=str, default=REDIS_BINARY)
     return parser
diff --git a/redisbench_admin/run_local/local_db.py b/redisbench_admin/run_local/local_db.py
@@ -35,6 +35,7 @@
 
 
 def local_db_spin(
+    binary,
     args,
     benchmark_config,
     clusterconfig,
@@ -79,6 +80,7 @@ def local_db_spin(
         cluster_api_enabled = True
         shard_host = "127.0.0.1"
         redis_processes, redis_conns = spin_up_local_redis_cluster(
+            binary,
             temporary_dir,
             shard_count,
             shard_host,
@@ -106,7 +108,7 @@ def local_db_spin(
     )
     if setup_type == "oss-standalone":
         redis_processes = spin_up_local_redis(
-            "redis-server",
+            binary,
             args.port,
             temporary_dir,
             local_module_file,
diff --git a/redisbench_admin/run_local/run_local.py b/redisbench_admin/run_local/run_local.py
@@ -178,11 +178,15 @@ def run_local_command_logic(args, project_name, project_version):
                                             setup_name, setup_type, shard_count
                                         )
                                     )
+                                    binary = args.redis_binary
+                                    if " " in binary:
+                                        binary = binary.split(" ")
                                     (
                                         cluster_api_enabled,
                                         redis_conns,
                                         redis_processes,
                                     ) = local_db_spin(
+                                        binary,
                                         args,
                                         benchmark_config,
                                         clusterconfig,

Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,7 @@`
`52`	`52`	`)`
`53`	`53`
`54`	`54`	`BENCHMARK_REPETITIONS = int(os.getenv("BENCHMARK_REPETITIONS", 1))`
	`55`	`+REDIS_BINARY = os.getenv("REDIS_BINARY", "redis-server")`
`55`	`56`
`56`	`57`
`57`	`58`	`def prepare_benchmark_parameters(`
Original file line number	Diff line number	Diff line change
`@@ -5,9 +5,11 @@`
`5`	`5`	`#`
`6`	`6`
`7`	`7`	`from redisbench_admin.run.args import common_run_args`
	`8`	`+from redisbench_admin.run.common import REDIS_BINARY`
`8`	`9`
`9`	`10`
`10`	`11`	`def create_run_local_arguments(parser):`
`11`	`12`	`parser = common_run_args(parser)`
`12`	`13`	`parser.add_argument("--port", type=int, default=6379)`
	`14`	`+ parser.add_argument("--redis-binary", type=str, default=REDIS_BINARY)`
`13`	`15`	`return parser`
Original file line number	Diff line number	Diff line change
`@@ -178,11 +178,15 @@ def run_local_command_logic(args, project_name, project_version):`
`178`	`178`	`setup_name, setup_type, shard_count`
`179`	`179`	`)`
`180`	`180`	`)`
	`181`	`+ binary = args.redis_binary`
	`182`	`+ if " " in binary:`
	`183`	`+ binary = binary.split(" ")`
`181`	`184`	`(`
`182`	`185`	`cluster_api_enabled,`
`183`	`186`	`redis_conns,`
`184`	`187`	`redis_processes,`
`185`	`188`	`) = local_db_spin(`
	`189`	`+ binary,`
`186`	`190`	`args,`
`187`	`191`	`benchmark_config,`
`188`	`192`	`clusterconfig,`