Merge pull request MaterializeInc#23984 from nrainer-materialize/test…

…s/handle-accepted-regressions-in-release-builds

tests: handle accepted regressions in release builds

misc/python/materialize/docker.py

@@ -13,6 +13,8 @@
 from materialize.mz_version import MzVersion
 
 EXISTENCE_OF_IMAGE_NAMES_FROM_EARLIER_CHECK: dict[str, bool] = dict()
+IMAGE_TAG_OF_VERSION_PREFIX = "v"
+IMAGE_TAG_OF_COMMIT_PREFIX = "devel-"
 
 
 def image_of_release_version_exists(version: MzVersion) -> bool:
@@ -58,8 +60,31 @@ def mz_image_tag_exists(image_tag: str) -> bool:
 
 
 def commit_to_image_tag(commit_hash: str) -> str:
-    return f"devel-{commit_hash}"
+    return f"{IMAGE_TAG_OF_COMMIT_PREFIX}{commit_hash}"
 
 
 def version_to_image_tag(version: MzVersion) -> str:
     return str(version)
+
+
+def is_image_tag_of_version(image_tag: str) -> bool:
+    return image_tag.startswith(IMAGE_TAG_OF_VERSION_PREFIX)
+
+
+def is_image_tag_of_commit(image_tag: str) -> bool:
+    return image_tag.startswith(IMAGE_TAG_OF_COMMIT_PREFIX)
+
+
+def get_version_from_image_tag(image_tag: str) -> str:
+    assert image_tag.startswith(IMAGE_TAG_OF_VERSION_PREFIX)
+    # image tag is equal to the version
+    return image_tag
+
+
+def get_mz_version_from_image_tag(image_tag: str) -> MzVersion:
+    return MzVersion.parse_mz(get_version_from_image_tag(image_tag))
+
+
+def get_commit_from_image_tag(image_tag: str) -> str:
+    assert image_tag.startswith(IMAGE_TAG_OF_COMMIT_PREFIX)
+    return image_tag.removeprefix(IMAGE_TAG_OF_COMMIT_PREFIX)

misc/python/materialize/scalability/comparison_outcome.py

@@ -14,6 +14,7 @@
     DfTotalsExtended,
     concat_df_totals_extended,
 )
+from materialize.scalability.endpoint import Endpoint
 from materialize.scalability.scalability_change import (
     Regression,
     ScalabilityChange,
@@ -29,6 +30,7 @@ def __init__(
         self.significant_improvements: list[ScalabilityImprovement] = []
         self.regression_df = DfTotalsExtended()
         self.significant_improvement_df = DfTotalsExtended()
+        self.endpoints_with_regressions: set[Endpoint] = set()
 
     def has_regressions(self) -> bool:
         assert len(self.regressions) == self.regression_df.length()
@@ -41,6 +43,9 @@ def has_significant_improvements(self) -> bool:
         )
         return len(self.significant_improvements) > 0
 
+    def has_scalability_changes(self) -> bool:
+        return self.has_regressions() or self.has_significant_improvements()
+
     def __str__(self) -> str:
         return f"{len(self.regressions)} regressions, {len(self.significant_improvements)} significant improvements"
 
@@ -60,17 +65,34 @@ def _to_description(self, entries: Sequence[ScalabilityChange]) -> str:
         return "\n".join(f"* {x}" for x in entries)
 
     def merge(self, other: ComparisonOutcome) -> None:
-        self.regressions.extend(other.regressions)
-        self.significant_improvements.extend(other.significant_improvements)
-        self.append_regression_df(other.regression_df)
-        self.append_significant_improvement_df(other.significant_improvement_df)
+        self.append_regressions(
+            other.regressions,
+            other.significant_improvements,
+            other.regression_df,
+            other.significant_improvement_df,
+        )
+
+    def append_regressions(
+        self,
+        regressions: list[Regression],
+        significant_improvements: list[ScalabilityImprovement],
+        regression_df: DfTotalsExtended,
+        significant_improvement_df: DfTotalsExtended,
+    ) -> None:
+        self.regressions.extend(regressions)
+        self.significant_improvements.extend(significant_improvements)
+        self._append_regression_df(regression_df)
+        self._append_significant_improvement_df(significant_improvement_df)
+
+        for regression in regressions:
+            self.endpoints_with_regressions.add(regression.endpoint)
 
-    def append_regression_df(self, regressions_data: DfTotalsExtended) -> None:
+    def _append_regression_df(self, regressions_data: DfTotalsExtended) -> None:
         self.regression_df = concat_df_totals_extended(
             [self.regression_df, regressions_data]
         )
 
-    def append_significant_improvement_df(
+    def _append_significant_improvement_df(
         self, significant_improvements_data: DfTotalsExtended
     ) -> None:
         self.significant_improvement_df = concat_df_totals_extended(

misc/python/materialize/scalability/endpoint.py

@@ -16,6 +16,9 @@ class Endpoint:
 
     _version: str | None = None
 
+    def __init__(self, specified_target: str):
+        self._specified_target = specified_target
+
     def sql_connection(self) -> psycopg.connection.Connection[tuple[Any, ...]]:
         conn = psycopg.connect(self.url())
         conn.autocommit = True
@@ -26,6 +29,12 @@ def url(self) -> str:
             f"postgresql://{self.user()}:{self.password()}@{self.host()}:{self.port()}"
         )
 
+    def specified_target(self) -> str:
+        return self._specified_target
+
+    def resolved_target(self) -> str:
+        return self.specified_target()
+
     def host(self) -> str:
         raise NotImplementedError
 

misc/python/materialize/scalability/endpoints.py

@@ -17,11 +17,17 @@
 
 POSTGRES_ENDPOINT_NAME = "postgres"
 
+TARGET_MATERIALIZE_LOCAL = "local"
+TARGET_MATERIALIZE_REMOTE = "remote"
+TARGET_POSTGRES = "postgres"
+TARGET_HEAD = "HEAD"
+
 
 class MaterializeRemote(Endpoint):
     """Connect to a remote Materialize instance using a psql URL"""
 
     def __init__(self, materialize_url: str) -> None:
+        super().__init__(specified_target=TARGET_MATERIALIZE_REMOTE)
         self.materialize_url = materialize_url
 
     def url(self) -> str:
@@ -38,7 +44,7 @@ class PostgresContainer(Endpoint):
     def __init__(self, composition: Composition) -> None:
         self.composition = composition
         self._port: int | None = None
-        super().__init__()
+        super().__init__(specified_target=TARGET_POSTGRES)
 
     def host(self) -> str:
         return "localhost"
@@ -67,6 +73,9 @@ def __str__(self) -> str:
 
 
 class MaterializeNonRemote(Endpoint):
+    def __init__(self, specified_target: str):
+        super().__init__(specified_target)
+
     def host(self) -> str:
         return "localhost"
 
@@ -92,6 +101,9 @@ def lift_limits(self) -> None:
 class MaterializeLocal(MaterializeNonRemote):
     """Connect to a Materialize instance running on the local host"""
 
+    def __init__(self) -> None:
+        super().__init__(specified_target=TARGET_MATERIALIZE_LOCAL)
+
     def port(self) -> int:
         return 6875
 
@@ -110,6 +122,7 @@ def __init__(
         self,
         composition: Composition,
         specified_target: str,
+        resolved_target: str,
         image: str | None = None,
         alternative_image: str | None = None,
     ) -> None:
@@ -119,8 +132,11 @@ def __init__(
             alternative_image if image != alternative_image else None
         )
         self._port: int | None = None
-        self.specified_target = specified_target
-        super().__init__()
+        self._resolved_target = resolved_target
+        super().__init__(specified_target)
+
+    def resolved_target(self) -> str:
+        return self._resolved_target
 
     def port(self) -> int:
         assert self._port is not None

misc/python/materialize/scalability/regression_assessment.py

@@ -0,0 +1,101 @@
+# Copyright Materialize, Inc. and contributors. All rights reserved.
+#
+# Use of this software is governed by the Business Source License
+# included in the LICENSE file at the root of this repository.
+#
+# As of the Change Date specified in that file, in accordance with
+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0.
+from __future__ import annotations
+
+from materialize.docker import (
+    get_mz_version_from_image_tag,
+    is_image_tag_of_version,
+)
+from materialize.mz_version import MzVersion
+from materialize.scalability.comparison_outcome import ComparisonOutcome
+from materialize.scalability.endpoint import Endpoint
+from materialize.version_list import (
+    ANCESTOR_OVERRIDES_FOR_SCALABILITY_REGRESSIONS,
+    get_commits_of_accepted_regressions_between_versions,
+)
+
+
+class RegressionAssessment:
+    def __init__(
+        self,
+        baseline_endpoint: Endpoint | None,
+        comparison_outcome: ComparisonOutcome,
+    ):
+        self.baseline_endpoint = baseline_endpoint
+        self.comparison_outcome = comparison_outcome
+        self.endpoints_with_regressions_and_justifications: dict[
+            Endpoint, str | None
+        ] = {}
+        self.check_targets()
+        assert len(comparison_outcome.endpoints_with_regressions) == len(
+            self.endpoints_with_regressions_and_justifications
+        )
+
+    def has_comparison_target(self) -> bool:
+        return self.baseline_endpoint is not None
+
+    def has_regressions(self) -> bool:
+        return self.comparison_outcome.has_regressions()
+
+    def has_unjustified_regressions(self):
+        return any(
+            justification is None
+            for justification in self.endpoints_with_regressions_and_justifications.values()
+        )
+
+    def check_targets(self) -> None:
+        if self.baseline_endpoint is None:
+            return
+
+        if not self.comparison_outcome.has_regressions():
+            return
+
+        if not self._endpoint_references_release_version(self.baseline_endpoint):
+            # justified regressions require a version as comparison target
+            self._mark_all_targets_with_regressions_as_unjustified()
+            return
+
+        baseline_version = get_mz_version_from_image_tag(
+            self.baseline_endpoint.resolved_target()
+        )
+
+        for endpoint in self.comparison_outcome.endpoints_with_regressions:
+            if not self._endpoint_references_release_version(endpoint):
+                continue
+
+            endpoint_version = get_mz_version_from_image_tag(endpoint.resolved_target())
+
+            if baseline_version >= endpoint_version:
+                # not supported, should not be a relevant case
+                continue
+
+            commits_with_accepted_regressions = (
+                get_commits_of_accepted_regressions_between_versions(
+                    ANCESTOR_OVERRIDES_FOR_SCALABILITY_REGRESSIONS,
+                    since_version_exclusive=baseline_version,
+                    to_version_inclusive=endpoint_version,
+                )
+            )
+
+            if len(commits_with_accepted_regressions) > 0:
+                self.endpoints_with_regressions_and_justifications[
+                    endpoint
+                ] = ", ".join(commits_with_accepted_regressions)
+            else:
+                self.endpoints_with_regressions_and_justifications[endpoint] = None
+
+    def _mark_all_targets_with_regressions_as_unjustified(self) -> None:
+        for endpoint in self.comparison_outcome.endpoints_with_regressions:
+            self.endpoints_with_regressions_and_justifications[endpoint] = None
+
+    def _endpoint_references_release_version(self, endpoint: Endpoint) -> bool:
+        target = endpoint.resolved_target()
+        return is_image_tag_of_version(target) and MzVersion.is_valid_version_string(
+            target
+        )

misc/python/materialize/scalability/result_analyzers.py

@@ -60,10 +60,10 @@ def perform_comparison_in_workload(
             workload_name,
             other_endpoint,
         )
-        comparison_outcome.regressions.extend(regressions)
-        comparison_outcome.significant_improvements.extend(improvements)
-        comparison_outcome.append_regression_df(entries_worse_than_threshold)
-        comparison_outcome.append_significant_improvement_df(
-            entries_better_than_threshold
+        comparison_outcome.append_regressions(
+            regressions,
+            improvements,
+            entries_worse_than_threshold,
+            entries_better_than_threshold,
         )
         return comparison_outcome

misc/python/materialize/version_list.py

@@ -391,6 +391,35 @@ def is_valid_release_image(version: MzVersion) -> bool:
     return docker.image_of_release_version_exists(version)
 
 
+def get_commits_of_accepted_regressions_between_versions(
+    ancestor_overrides: dict[str, MzVersion],
+    since_version_exclusive: MzVersion,
+    to_version_inclusive: MzVersion,
+) -> list[str]:
+    """
+    Get commits of accepted regressions between both versions.
+    :param ancestor_overrides: one of #ANCESTOR_OVERRIDES_FOR_PERFORMANCE_REGRESSIONS, #ANCESTOR_OVERRIDES_FOR_SCALABILITY_REGRESSIONS, #ANCESTOR_OVERRIDES_FOR_CORRECTNESS_REGRESSIONS
+    :return: commits
+    """
+
+    assert since_version_exclusive <= to_version_inclusive
+
+    commits = []
+
+    for (
+        regression_introducing_commit,
+        first_version_with_regression,
+    ) in ancestor_overrides.items():
+        if (
+            since_version_exclusive
+            < first_version_with_regression
+            <= to_version_inclusive
+        ):
+            commits.append(regression_introducing_commit)
+
+    return commits
+
+
 class VersionsFromDocs:
     """Materialize versions as listed in doc/user/content/versions