From 6bb9855d7c17d1c3d916afcb383dc0a457ce79e0 Mon Sep 17 00:00:00 2001 From: Mike Alfare <13974384+mikealfare@users.noreply.github.com> Date: Fri, 11 Aug 2023 12:05:41 -0400 Subject: [PATCH] ADAP-761: Add retry logic to flaky MV tests to avoid "cannot open relation with OID" error (#569) (#574) * add retry logic to materialized view tests to avoid "could not open relation with OID" error * changie * update utility method name to be scope specific (cherry picked from commit cbb411d841c0cef5456c0f45fa0cdb1182beabaf) --- .../Under the Hood-20230808-141645.yaml | 7 ++ .../test_materialized_views.py | 76 ++++++++++++++++++- .../adapter/materialized_view_tests/utils.py | 20 ++++- 3 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 .changes/unreleased/Under the Hood-20230808-141645.yaml diff --git a/.changes/unreleased/Under the Hood-20230808-141645.yaml b/.changes/unreleased/Under the Hood-20230808-141645.yaml new file mode 100644 index 000000000..051bf0e33 --- /dev/null +++ b/.changes/unreleased/Under the Hood-20230808-141645.yaml @@ -0,0 +1,7 @@ +kind: Under the Hood +body: Update flaky MV tests to use retry logic to avoid "cannot open relation with + OID" error +time: 2023-08-08T14:16:45.227308-04:00 +custom: + Author: mikealfare + Issue: "569" diff --git a/tests/functional/adapter/materialized_view_tests/test_materialized_views.py b/tests/functional/adapter/materialized_view_tests/test_materialized_views.py index d4c3e8a11..7bb5bcee6 100644 --- a/tests/functional/adapter/materialized_view_tests/test_materialized_views.py +++ b/tests/functional/adapter/materialized_view_tests/test_materialized_views.py @@ -12,13 +12,14 @@ MaterializedViewChangesFailMixin, ) from dbt.tests.adapter.materialized_view.files import MY_TABLE, MY_VIEW -from dbt.tests.util import get_model_file, set_model_file +from dbt.tests.util import assert_message_in_logs, get_model_file, set_model_file from tests.functional.adapter.materialized_view_tests.utils import ( query_autorefresh, query_dist, query_relation_type, query_sort, + run_dbt_and_capture_with_retries_redshift_mv, ) @@ -61,6 +62,14 @@ def query_row_count(project, relation: BaseRelation) -> int: def query_relation_type(project, relation: BaseRelation) -> Optional[str]: return query_relation_type(project, relation) + def test_materialized_view_create_idempotent(self, project, my_materialized_view): + # setup creates it once; verify it's there and run once + assert self.query_relation_type(project, my_materialized_view) == "materialized_view" + run_dbt_and_capture_with_retries_redshift_mv( + ["run", "--models", my_materialized_view.identifier] + ) + assert self.query_relation_type(project, my_materialized_view) == "materialized_view" + @pytest.mark.skip( "The current implementation does not support overwriting materialized views with tables." ) @@ -120,16 +129,77 @@ def check_state_replace_change_is_applied(project, materialized_view): class TestRedshiftMaterializedViewChangesApply( RedshiftMaterializedViewChanges, MaterializedViewChangesApplyMixin ): - pass + def test_change_is_applied_via_alter(self, project, my_materialized_view): + self.check_start_state(project, my_materialized_view) + + self.change_config_via_alter(project, my_materialized_view) + _, logs = run_dbt_and_capture_with_retries_redshift_mv( + ["--debug", "run", "--models", my_materialized_view.name] + ) + + self.check_state_alter_change_is_applied(project, my_materialized_view) + + assert_message_in_logs(f"Applying ALTER to: {my_materialized_view}", logs) + assert_message_in_logs(f"Applying REPLACE to: {my_materialized_view}", logs, False) + + def test_change_is_applied_via_replace(self, project, my_materialized_view): + self.check_start_state(project, my_materialized_view) + + self.change_config_via_alter(project, my_materialized_view) + self.change_config_via_replace(project, my_materialized_view) + _, logs = run_dbt_and_capture_with_retries_redshift_mv( + ["--debug", "run", "--models", my_materialized_view.name] + ) + + self.check_state_alter_change_is_applied(project, my_materialized_view) + self.check_state_replace_change_is_applied(project, my_materialized_view) + + assert_message_in_logs(f"Applying REPLACE to: {my_materialized_view}", logs) class TestRedshiftMaterializedViewChangesContinue( RedshiftMaterializedViewChanges, MaterializedViewChangesContinueMixin ): - pass + def test_change_is_not_applied_via_alter(self, project, my_materialized_view): + self.check_start_state(project, my_materialized_view) + + self.change_config_via_alter(project, my_materialized_view) + _, logs = run_dbt_and_capture_with_retries_redshift_mv( + ["--debug", "run", "--models", my_materialized_view.name] + ) + + self.check_start_state(project, my_materialized_view) + + assert_message_in_logs( + f"Configuration changes were identified and `on_configuration_change` was set" + f" to `continue` for `{my_materialized_view}`", + logs, + ) + assert_message_in_logs(f"Applying ALTER to: {my_materialized_view}", logs, False) + assert_message_in_logs(f"Applying REPLACE to: {my_materialized_view}", logs, False) + + def test_change_is_not_applied_via_replace(self, project, my_materialized_view): + self.check_start_state(project, my_materialized_view) + + self.change_config_via_alter(project, my_materialized_view) + self.change_config_via_replace(project, my_materialized_view) + _, logs = run_dbt_and_capture_with_retries_redshift_mv( + ["--debug", "run", "--models", my_materialized_view.name] + ) + + self.check_start_state(project, my_materialized_view) + + assert_message_in_logs( + f"Configuration changes were identified and `on_configuration_change` was set" + f" to `continue` for `{my_materialized_view}`", + logs, + ) + assert_message_in_logs(f"Applying ALTER to: {my_materialized_view}", logs, False) + assert_message_in_logs(f"Applying REPLACE to: {my_materialized_view}", logs, False) class TestRedshiftMaterializedViewChangesFail( RedshiftMaterializedViewChanges, MaterializedViewChangesFailMixin ): + # Note: using retries doesn't work when we expect `dbt_run` to fail pass diff --git a/tests/functional/adapter/materialized_view_tests/utils.py b/tests/functional/adapter/materialized_view_tests/utils.py index 5ca1b939b..bc172be69 100644 --- a/tests/functional/adapter/materialized_view_tests/utils.py +++ b/tests/functional/adapter/materialized_view_tests/utils.py @@ -1,6 +1,7 @@ -from typing import Optional +from typing import List, Optional from dbt.adapters.base.relation import BaseRelation +from dbt.tests.util import run_dbt_and_capture from dbt.adapters.redshift.relation import RedshiftRelation @@ -67,3 +68,20 @@ def query_autorefresh(project, relation: RedshiftRelation) -> bool: and trim(mv.db_name) ilike '{ relation.database }' """ return project.run_sql(sql, fetch="one")[0] + + +def run_dbt_and_capture_with_retries_redshift_mv(args: List[str], max_retries: int = 10): + """ + We need to retry `run_dbt` calls on Redshift because we get sporadic failures of the form: + + Database Error in model my_materialized_view (models/my_materialized_view.sql) + could not open relation with OID 14957412 + """ + retries = 0 + while retries < max_retries: + try: + # there's no point to using this with expect_pass=False + return run_dbt_and_capture(args, expect_pass=True) + except AssertionError: + retries += 1 + return None