From 2e5e7208e62f39eb390bfc44865909708f8f0da5 Mon Sep 17 00:00:00 2001 From: Hari Krishna Sunder Date: Thu, 30 Jan 2025 10:05:14 -0800 Subject: [PATCH] [#24732] YSQL: Refresh materialized view in-place during upgrade Summary: **Behavior before this change:** Non-concurrent refresh of a materialized view involves generating the new data in a new DocDB table, and then swapping the tables by replacing the `relfinenode` info of the matview. This involves 16 RPCs to yb-master (pg catalog) with 73 pgsql_write_batches. All of these are not allowed during the ysql major upgrade. Concurrent refresh of a supported materialized view involves generating the new data in temp table, followed by doing a merge of the new data with the old data already in the matview using another temp table. This involves 24 RPC calls to yb-master (pg catalog) with 241 pgsql_write_batches. However 23 of these RPCs are related to the temp tables (force_catalog_modifications set) which are allowed during ysql major upgrade. The only other RPC is related to modifying the `relispopulated` field. **Behavior with this change:** During ysql major upgrade changes to the `relispopulated` field is blocked. This is not an issue with concurrent refresh, since those are always blocked. The non-concurrent refresh of matview now generates the new data in a temporary table, and use the more expensive method of running `DELETE FROM ` followed by a `INSERT INTO SELECT * FROM `. The advantage of this method is that all pg catalog modifications are only done to the temp table, which is allowed during the upgrade. The performance tradeoff is acceptable, since we only expect small matviews to be refreshed frequently, hences needed during the upgrade, whereas the bigger ones can wait till after the upgrade to be refreshed. This behavior is enabled by `yb_refresh_matview_in_place` or `yb_major_version_upgrade_compatibility>0`. Concurrent refresh has not been modified, since it only creates temporary tables, and does not involve any other catalog modifications. Fixes #24732 Jira: DB-13813 Test Plan: YsqlMajorUpgradeMatviewTest.TestMatView Reviewers: telgersma, smishra, myang Reviewed By: telgersma, myang Subscribers: svc_phabricator, ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D41504 --- src/postgres/src/backend/commands/createas.c | 3 +- src/postgres/src/backend/commands/matview.c | 96 ++++++++++++++++-- src/postgres/src/backend/utils/misc/guc.c | 23 +++++ src/postgres/src/include/commands/matview.h | 4 +- src/yb/integration-tests/CMakeLists.txt | 1 + .../upgrade-tests/upgrade_test_base.cc | 13 ++- .../ysql_major_upgrade_ddl_blocking-test.cc | 17 +--- .../ysql_major_upgrade_matview-test.cc | 99 +++++++++++++++++++ src/yb/yql/pggate/util/ybc_guc.cc | 4 + src/yb/yql/pggate/util/ybc_guc.h | 4 + src/yb/yql/pgwrapper/pg_wrapper.cc | 5 + 11 files changed, 246 insertions(+), 23 deletions(-) create mode 100644 src/yb/integration-tests/upgrade-tests/ysql_major_upgrade_matview-test.cc diff --git a/src/postgres/src/backend/commands/createas.c b/src/postgres/src/backend/commands/createas.c index aacd3a0a498..2dd3416efde 100644 --- a/src/postgres/src/backend/commands/createas.c +++ b/src/postgres/src/backend/commands/createas.c @@ -558,7 +558,8 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) * going to fill it; otherwise, no change needed. */ if (is_matview && !into->skipData) - SetMatViewPopulatedState(intoRelationDesc, true); + SetMatViewPopulatedState(intoRelationDesc, true, + false /* yb_in_place_refresh */ ); /* * Fill private fields of myState for use by later routines diff --git a/src/postgres/src/backend/commands/matview.c b/src/postgres/src/backend/commands/matview.c index 46a6fe24f2f..3d7b81edb17 100644 --- a/src/postgres/src/backend/commands/matview.c +++ b/src/postgres/src/backend/commands/matview.c @@ -79,6 +79,9 @@ static bool is_usable_unique_index(Relation indexRel); static void OpenMatViewIncrementalMaintenance(void); static void CloseMatViewIncrementalMaintenance(void); +static bool yb_needs_in_place_refresh(); +static void yb_refresh_in_place_update(Relation matviewRel, Oid tempOid); + /* * SetMatViewPopulatedState * Mark a materialized view as populated, or not. @@ -86,7 +89,8 @@ static void CloseMatViewIncrementalMaintenance(void); * NOTE: caller must be holding an appropriate lock on the relation. */ void -SetMatViewPopulatedState(Relation relation, bool newstate) +SetMatViewPopulatedState(Relation relation, bool newstate, + bool yb_in_place_refresh) { Relation pgrel; HeapTuple tuple; @@ -105,6 +109,17 @@ SetMatViewPopulatedState(Relation relation, bool newstate) elog(ERROR, "cache lookup failed for relation %u", RelationGetRelid(relation)); + if (yb_in_place_refresh) + { + if (((Form_pg_class) GETSTRUCT(tuple))->relispopulated != newstate) + elog(ERROR, "Cannot change the populated state of a materialized " + "view when in place refresh is enabled"); + + heap_freetuple(tuple); + table_close(pgrel, RowExclusiveLock); + return; + } + ((Form_pg_class) GETSTRUCT(tuple))->relispopulated = newstate; CatalogTupleUpdate(pgrel, &tuple->t_self, tuple); @@ -160,6 +175,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, int save_sec_context; int save_nestlevel; ObjectAddress address; + bool yb_in_place_refresh = yb_needs_in_place_refresh(); /* Determine strength of lock needed. */ concurrent = stmt->concurrent; @@ -283,10 +299,10 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, * Tentatively mark the matview as populated or not (this will roll back * if we fail later). */ - SetMatViewPopulatedState(matviewRel, !stmt->skipData); + SetMatViewPopulatedState(matviewRel, !stmt->skipData, yb_in_place_refresh); /* Concurrent refresh builds new data in temp tablespace, and does diff. */ - if (concurrent) + if (concurrent || yb_in_place_refresh) { tableSpace = GetDefaultTablespace(RELPERSISTENCE_TEMP, false); relpersistence = RELPERSISTENCE_TEMP; @@ -297,6 +313,13 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, relpersistence = matviewRel->rd_rel->relpersistence; } + /* + * Required to allow the creation for the temp table since we directly + * call make_new_heap instead of going through DefineRelation. + */ + if (yb_in_place_refresh) + YBCDdlEnableForceCatalogModification(); + /* * Create the transient table that will receive the regenerated data. Lock * it against access by any other process until commit (by which time it @@ -314,14 +337,17 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, processed = refresh_matview_datafill(dest, dataQuery, queryString); /* Make the matview match the newly generated data. */ - if (concurrent) + if (concurrent || yb_in_place_refresh) { int old_depth = matview_maintenance_depth; PG_TRY(); { - refresh_by_match_merge(matviewOid, OIDNewHeap, relowner, - save_sec_context); + if (!concurrent) + yb_refresh_in_place_update(matviewRel, OIDNewHeap); + else + refresh_by_match_merge(matviewOid, OIDNewHeap, relowner, + save_sec_context); } PG_CATCH(); { @@ -1050,3 +1076,61 @@ CloseMatViewIncrementalMaintenance(void) matview_maintenance_depth--; Assert(matview_maintenance_depth >= 0); } + +static bool +yb_needs_in_place_refresh() +{ + return yb_refresh_matview_in_place || + yb_major_version_upgrade_compatibility > 0; +} + +/* + * yb_refresh_in_place_update + * Refresh the matview by using the same heap/DocDB table. Deletes all rows from + * the table and inserts the data from the temp table. + */ +static void +yb_refresh_in_place_update(Relation matviewRel, Oid tempOid) +{ + Assert(IsYugaByteEnabled()); + + StringInfoData querybuf; + Relation tempRel; + char *matviewname; + char *tempname; + + initStringInfo(&querybuf); + matviewname = quote_qualified_identifier(get_namespace_name(RelationGetNamespace(matviewRel)), + RelationGetRelationName(matviewRel)); + tempRel = table_open(tempOid, NoLock); + tempname = quote_qualified_identifier(get_namespace_name(RelationGetNamespace(tempRel)), + RelationGetRelationName(tempRel)); + + if (SPI_connect() != SPI_OK_CONNECT) + elog(ERROR, "SPI_connect failed"); + + OpenMatViewIncrementalMaintenance(); + + appendStringInfo(&querybuf, "DELETE FROM %s", matviewname); + + if (SPI_exec(querybuf.data, 0) != SPI_OK_DELETE) + elog(ERROR, "SPI_exec failed: %s", querybuf.data); + + resetStringInfo(&querybuf); + appendStringInfo(&querybuf, "INSERT INTO %s SELECT * FROM %s", matviewname, + tempname); + + if (SPI_exec(querybuf.data, 0) != SPI_OK_INSERT) + elog(ERROR, "SPI_exec failed: %s", querybuf.data); + + CloseMatViewIncrementalMaintenance(); + table_close(tempRel, NoLock); + + resetStringInfo(&querybuf); + appendStringInfo(&querybuf, "DROP TABLE %s", tempname); + if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY) + elog(ERROR, "SPI_exec failed: %s", querybuf.data); + + if (SPI_finish() != SPI_OK_FINISH) + elog(ERROR, "SPI_finish failed"); +} diff --git a/src/postgres/src/backend/utils/misc/guc.c b/src/postgres/src/backend/utils/misc/guc.c index c5d6d4e3d5a..0f4575574cb 100644 --- a/src/postgres/src/backend/utils/misc/guc.c +++ b/src/postgres/src/backend/utils/misc/guc.c @@ -3142,6 +3142,17 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, + { + {"yb_refresh_matview_in_place", PGC_USERSET, CUSTOM_OPTIONS, + gettext_noop("Refresh materialized views in place."), + NULL, + GUC_NOT_IN_SAMPLE + }, + &yb_refresh_matview_in_place, + false, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL @@ -4995,6 +5006,18 @@ static struct config_int ConfigureNamesInt[] = NULL, NULL, NULL }, + { + {"yb_major_version_upgrade_compatibility", PGC_SIGHUP, CUSTOM_OPTIONS, + gettext_noop("The compatibility level to use during a YSQL Major version upgrade. " + "Allowed values are 0 and 11."), + NULL, + GUC_NOT_IN_SAMPLE + }, + &yb_major_version_upgrade_compatibility, + 0, 0, INT_MAX, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL diff --git a/src/postgres/src/include/commands/matview.h b/src/postgres/src/include/commands/matview.h index a067da39d29..bbe451828ae 100644 --- a/src/postgres/src/include/commands/matview.h +++ b/src/postgres/src/include/commands/matview.h @@ -20,8 +20,8 @@ #include "tcop/dest.h" #include "utils/relcache.h" - -extern void SetMatViewPopulatedState(Relation relation, bool newstate); +extern void SetMatViewPopulatedState(Relation relation, bool newstate, + bool yb_in_place_refresh); extern ObjectAddress ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, ParamListInfo params, QueryCompletion *qc); diff --git a/src/yb/integration-tests/CMakeLists.txt b/src/yb/integration-tests/CMakeLists.txt index b9881560eae..7e0f1149c0f 100644 --- a/src/yb/integration-tests/CMakeLists.txt +++ b/src/yb/integration-tests/CMakeLists.txt @@ -272,6 +272,7 @@ ADD_YB_TEST(upgrade-tests/pg15_upgrade-test) ADD_YB_TEST(upgrade-tests/pg15_upgrade_pgregress-test) ADD_YB_TEST(upgrade-tests/ysql_major_upgrade_rpcs-test) ADD_YB_TEST(upgrade-tests/ysql_major_upgrade_ddl_blocking-test) +ADD_YB_TEST(upgrade-tests/ysql_major_upgrade_matview-test) set(YB_TEST_LINK_LIBS_SAVED ${YB_TEST_LINK_LIBS}) set(YB_TEST_LINK_LIBS ${YB_TEST_LINK_LIBS} cassandra) diff --git a/src/yb/integration-tests/upgrade-tests/upgrade_test_base.cc b/src/yb/integration-tests/upgrade-tests/upgrade_test_base.cc index 9c5c5352860..c573291e873 100644 --- a/src/yb/integration-tests/upgrade-tests/upgrade_test_base.cc +++ b/src/yb/integration-tests/upgrade-tests/upgrade_test_base.cc @@ -319,8 +319,12 @@ Status UpgradeTestBase::StartClusterInOldVersion(const ExternalMiniClusterOption current_version_info_.ysql_major_version(); if (IsYsqlMajorVersionUpgrade()) { - // YB_TODO: Remove when support for expression pushdown in mixed mode is implemented. + // TODO: Remove when support for expression pushdown in mixed mode is implemented. RETURN_NOT_OK(cluster_->AddAndSetExtraFlag("ysql_yb_enable_expression_pushdown", "false")); + + // TODO: Enable after flag is backported to older version. + // RETURN_NOT_OK(cluster_->AddAndSetExtraFlag("ysql_yb_major_version_upgrade_compatibility", + // "11")); } return Status::OK(); @@ -508,6 +512,13 @@ Status UpgradeTestBase::FinalizeYsqlMajorCatalogUpgrade() { return StatusFromPB(resp.error().status()); } + if (IsYsqlMajorVersionUpgrade()) { + // TODO: Remove when support for expression pushdown in mixed mode is implemented. + RETURN_NOT_OK(cluster_->AddAndSetExtraFlag("ysql_yb_enable_expression_pushdown", "true")); + + RETURN_NOT_OK(cluster_->AddAndSetExtraFlag("ysql_yb_major_version_upgrade_compatibility", "0")); + } + return Status::OK(); } diff --git a/src/yb/integration-tests/upgrade-tests/ysql_major_upgrade_ddl_blocking-test.cc b/src/yb/integration-tests/upgrade-tests/ysql_major_upgrade_ddl_blocking-test.cc index 701d274dc1c..18bb307c4cc 100644 --- a/src/yb/integration-tests/upgrade-tests/ysql_major_upgrade_ddl_blocking-test.cc +++ b/src/yb/integration-tests/upgrade-tests/ysql_major_upgrade_ddl_blocking-test.cc @@ -60,19 +60,10 @@ class YsqlMajorUpgradeDdlBlockingTest : public Pg15UpgradeTestBase { if (!error_expected) { RETURN_NOT_OK(status); } else { - if (upgrade_state_ == UpgradeState::kAfterUpgrade) { - // Depending on the cleanup state we may get different errors after the upgrade. - SCHECK( - !status.ok() && - (status.message().ToString().find(kExpectedDdlError) != std::string::npos || - status.message().ToString().find("unknown_table_name") != std::string::npos), - IllegalState, "Unexpected status: ", status.ToString()); - } else { - SCHECK( - !status.ok() && - status.message().ToString().find(kExpectedDdlError) != std::string::npos, - IllegalState, "Unexpected status: ", status.ToString()); - } + SCHECK( + !status.ok() && + status.message().ToString().find(kExpectedDdlError) != std::string::npos, + IllegalState, "Unexpected status: ", status.ToString()); } } return Status::OK(); diff --git a/src/yb/integration-tests/upgrade-tests/ysql_major_upgrade_matview-test.cc b/src/yb/integration-tests/upgrade-tests/ysql_major_upgrade_matview-test.cc new file mode 100644 index 00000000000..aaa490d51d7 --- /dev/null +++ b/src/yb/integration-tests/upgrade-tests/ysql_major_upgrade_matview-test.cc @@ -0,0 +1,99 @@ +// Copyright (c) YugabyteDB, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software distributed under the License +// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions and limitations +// under the License. +// + +#include "yb/integration-tests/upgrade-tests/pg15_upgrade_test_base.h" + +#include "yb/yql/pgwrapper/libpq_utils.h" + +namespace yb { + +static constexpr auto kTableName = "tbl1"; +static constexpr auto kNormalMatviewName = "mv1"; +static constexpr auto kIndexedMatViewName = "mv2"; + +class YsqlMajorUpgradeMatviewTest : public Pg15UpgradeTestBase { + public: + void SetUp() override { + Pg15UpgradeTestBase::SetUp(); + if (Test::IsSkipped()) { + return; + } + + auto conn = ASSERT_RESULT(CreateConnToTs(std::nullopt)); + ASSERT_OK(conn.ExecuteFormat("CREATE TABLE $0(a int, b int)", kTableName)); + ASSERT_OK(conn.ExecuteFormat( + "CREATE MATERIALIZED VIEW $0 AS SELECT a FROM $1", kNormalMatviewName, kTableName)); + ASSERT_OK(conn.ExecuteFormat( + "CREATE MATERIALIZED VIEW $0 AS SELECT a FROM $1", kIndexedMatViewName, kTableName)); + ASSERT_OK(conn.ExecuteFormat("CREATE UNIQUE INDEX idx1 ON $0(a)", kIndexedMatViewName)); + + ASSERT_OK(InsertDataRefreshAndValidate()); + } + + Status InsertDataRefreshAndValidate( + std::optional ts_id = std::nullopt, int row_count = 10) { + auto conn = VERIFY_RESULT(CreateConnToTs(ts_id)); + RETURN_NOT_OK(InsertData(conn, row_count)); + return RefreshMatviewsAndValidate(conn); + } + + Status InsertData(pgwrapper::PGConn& conn, int row_count) { + for (int i = 0; i < row_count; ++i) { + RETURN_NOT_OK(conn.ExecuteFormat("INSERT INTO $0 VALUES ($1, $1)", kTableName, ++num_rows_)); + } + return Status::OK(); + } + + Status RefreshMatviewsAndValidate(pgwrapper::PGConn& conn) { + RETURN_NOT_OK(conn.ExecuteFormat("REFRESH MATERIALIZED VIEW $0", kNormalMatviewName)); + RETURN_NOT_OK( + conn.ExecuteFormat("REFRESH MATERIALIZED VIEW CONCURRENTLY $0", kIndexedMatViewName)); + + auto table_data = VERIFY_RESULT(conn.FetchRows("SELECT a FROM tbl1 ORDER BY a")); + LOG(INFO) << "Table data: " << yb::ToString(table_data); + + auto normal_matview_data = VERIFY_RESULT(conn.FetchRows("SELECT a FROM mv1 ORDER BY a")); + SCHECK_EQ( + table_data, normal_matview_data, IllegalState, + Format("Normal matview data mismatch: $0", yb::ToString(normal_matview_data))); + + auto indexes_matview_data = VERIFY_RESULT(conn.FetchRows("SELECT a FROM mv2 ORDER BY a")); + SCHECK_EQ( + table_data, indexes_matview_data, IllegalState, + Format("Indexed matview data mismatch: $0", yb::ToString(indexes_matview_data))); + + return Status::OK(); + } + + private: + uint32 num_rows_ = 0; +}; + +TEST_F(YsqlMajorUpgradeMatviewTest, TestMatView) { + ASSERT_OK(UpgradeClusterToMixedMode()); + + ASSERT_NOK(InsertDataRefreshAndValidate(kMixedModeTserverPg11)); + + // TODO: Pending backport + ASSERT_OK(cluster_->SetFlag( + cluster_->tablet_server(kMixedModeTserverPg15), "ysql_yb_major_version_upgrade_compatibility", + "11")); + + ASSERT_OK(InsertDataRefreshAndValidate(kMixedModeTserverPg15)); + + ASSERT_OK(FinalizeUpgradeFromMixedMode()); + + ASSERT_OK(InsertDataRefreshAndValidate()); +} + +} // namespace yb diff --git a/src/yb/yql/pggate/util/ybc_guc.cc b/src/yb/yql/pggate/util/ybc_guc.cc index a65cf38409d..8eaa8ab69c7 100644 --- a/src/yb/yql/pggate/util/ybc_guc.cc +++ b/src/yb/yql/pggate/util/ybc_guc.cc @@ -87,3 +87,7 @@ bool yb_allow_block_based_sampling_algorithm = true; // TODO(#24089): Once code duplication between yb_guc and ybc_util is removed, we should be able // to use YB_SAMPLING_ALGORITHM_BLOCK_BASED_SAMPLING instead of 1 and do it in one place. int32_t yb_sampling_algorithm = 1 /* YB_SAMPLING_ALGORITHM_BLOCK_BASED_SAMPLING */; + +bool yb_refresh_matview_in_place = false; + +int yb_major_version_upgrade_compatibility = 0; diff --git a/src/yb/yql/pggate/util/ybc_guc.h b/src/yb/yql/pggate/util/ybc_guc.h index d7d09ba8d18..320dae791fe 100644 --- a/src/yb/yql/pggate/util/ybc_guc.h +++ b/src/yb/yql/pggate/util/ybc_guc.h @@ -232,6 +232,10 @@ extern int yb_read_after_commit_visibility; extern bool yb_allow_block_based_sampling_algorithm; +extern bool yb_refresh_matview_in_place; + +extern int yb_major_version_upgrade_compatibility; + // Should be in sync with YsqlSamplingAlgorithm protobuf. typedef enum { YB_SAMPLING_ALGORITHM_FULL_TABLE_SCAN = 0, diff --git a/src/yb/yql/pgwrapper/pg_wrapper.cc b/src/yb/yql/pgwrapper/pg_wrapper.cc index 80601424a1a..cb6da8789df 100644 --- a/src/yb/yql/pgwrapper/pg_wrapper.cc +++ b/src/yb/yql/pgwrapper/pg_wrapper.cc @@ -331,6 +331,11 @@ DEFINE_NON_RUNTIME_PG_PREVIEW_FLAG(bool, yb_enable_query_diagnostics, false, "Enables the collection of query diagnostics data for YSQL queries, " "facilitating the creation of diagnostic bundles."); +DEFINE_RUNTIME_PG_FLAG(int32, yb_major_version_upgrade_compatibility, 0, + "The compatibility level to use during a YSQL Major version upgrade. Allowed values are 0 and " + "11."); +DEFINE_validator(ysql_yb_major_version_upgrade_compatibility, FLAG_IN_SET_VALIDATOR(0, 11)); + DECLARE_bool(enable_pg_cron); using gflags::CommandLineFlagInfo;