Skip to content

Commit

Permalink
[#24720] YSQL: Disable auto analyze on the database during restore
Browse files Browse the repository at this point in the history
Summary:
Auto analyze service should skip running ANALYZEs on a database that is being restored. Otherwise, serialization errors can occur between DDL statements in the dump script and ANALYZEs run by auto analyze in the background when a cluster has auto analyze service enabled. These serialization errors occur on the concurrent increments to the catalog version.

To support this, the `yb_disable_auto_analyze` GUC is introduced. By executing `ALTER DATABASE ... SET yb_disable_auto_analyze=on`, a user can stop auto analyze on all tables of the DB. The auto analyze service queries for the value of this GUC before running ANALYZE on tables that have crossed the required mutation threshold. To resume auto analyze again, the user should set the GUC to off.

With this change, ysql_dump will disable auto analyze on the target database before any DDLs are executed and re-enable it after all DDLs are done if:
(1) `--format p` (which is the default plain text output format) is used,
(2) `--include-yb-metadata` is specified and
(3) `--create` is not specified (this is because if output script also has to create the database, we need to disable auto analyze after the creation. This will be done in a follow-up revision).

This change also disables auto analyze during a YSQL major version upgrade.

## Cross-version backup/restore compatibility:
To maintain backwards compatibility i.e., when running restore on a db version without this GUC, the restore script checks for the presence of the GUC before setting it on the database using ALTER DATABASE.
Jira: DB-13797

Test Plan:
./yb_build.sh release --gtest_filter PgAutoAnalyzeTest.DisableAndReEnableAutoAnalyze
./yb_build.sh release --cxx-test yb-backup-cross-feature-test --gtest-filter YBBackupTest.TestAutoAnalyzeEnabledDuringRestore
./yb_build.sh release --java-test org.yb.pgsql.TestYsqlDump
./yb_build.sh release --java-test org.yb.pgsql.TestYsqlUpgrade

Reviewers: mihnea, hsunder, yguan, sanketh

Reviewed By: hsunder

Subscribers: sanketh, smishra, ybase, loginov, yql

Differential Revision: https://phorge.dev.yugabyte.com/D40066
  • Loading branch information
yifanguan authored and pkj415 committed Mar 3, 2025
1 parent 42c3155 commit a842915
Show file tree
Hide file tree
Showing 19 changed files with 366 additions and 49 deletions.
32 changes: 32 additions & 0 deletions src/postgres/src/backend/utils/misc/guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ extern void YBCAssignTransactionPriorityUpperBound(double newval, void *extra);
extern double YBCGetTransactionPriority();
extern YbcTxnPriorityRequirement YBCGetTransactionPriorityType();
static bool yb_check_no_txn(int *newval, void **extra, GucSource source);
static bool yb_disable_auto_analyze_check_hook(bool *newval, void **extra, GucSource source);

static void assign_yb_pg_batch_detection_mechanism(int new_value, void *extra);
static void assign_ysql_upgrade_mode(bool newval, void *extra);
Expand Down Expand Up @@ -3192,6 +3193,19 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},

{
{"yb_disable_auto_analyze", PGC_USERSET, CUSTOM_OPTIONS,
gettext_noop("Run 'ALTER DATABASE <name> SET yb_disable_auto_analyze=on' to disable auto "
"analyze on that database. Set it to off to resume auto analyze. Setting this GUC via "
"any other method is not allowed."),
NULL,
GUC_NOT_IN_SAMPLE
},
&yb_disable_auto_analyze,
false,
yb_disable_auto_analyze_check_hook, NULL, NULL
},

/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL
Expand Down Expand Up @@ -15464,5 +15478,23 @@ assign_tcmalloc_sample_period(int newval, void *extra)
YBCSetTCMallocSamplingPeriod(newval);
}

static bool
yb_disable_auto_analyze_check_hook(bool *newval, void **extra, GucSource source)
{
/*
* PGC_S_DEFAULT means that GUCs are being initialized during startup. PGC_S_TEST will be seen
* when GUCs are being tested for their setting when applying the setting on a per-database
* level. In both cases, we want to allow the setting to be changed.
*/
if (source == PGC_S_DEFAULT || source == PGC_S_TEST)
return true;

if (source != PGC_S_DATABASE)
{
GUC_check_errmsg("Can only be set on a database level using ALTER DATABASE SET. Current source: %s", GucSource_Names[source]);
return false;
}
return true;
}

#include "guc-file.c"
32 changes: 32 additions & 0 deletions src/postgres/src/bin/pg_dump/pg_backup_archiver.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,17 @@ static void inhibit_data_for_failed_table(ArchiveHandle *AH, TocEntry *te);

static void StrictNamesCheck(RestoreOptions *ropt);

/*
* Set the yb_disable_auto_analyze GUC on the database to disable auto analyze. For backwards
* compatiblity, check for the presence of the GUC before setting it.
*/
static const char *yb_disable_auto_analyze_cmd =
"DO $$\n"
"BEGIN\n"
"IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN\n"
"EXECUTE format('ALTER DATABASE %%I SET yb_disable_auto_analyze TO %s', current_database());\n"
"END IF;\n"
"END $$;\n";

/*
* Allocate a new DumpOptions block containing all default values.
Expand Down Expand Up @@ -733,6 +744,13 @@ RestoreArchive(Archive *AHX)
}
}

if (AH->public.dopt->include_yb_metadata && !AH->public.ropt->createDB)
{
ahprintf(AH, "-- YB: re-enable auto analyze after all catalog changes\n");
ahprintf(AH, yb_disable_auto_analyze_cmd, "off");
ahprintf(AH, "\n");
}

if (ropt->single_txn)
{
if (AH->connection)
Expand Down Expand Up @@ -3144,6 +3162,20 @@ _doSetFixedOutputState(ArchiveHandle *AH)
"\\else\n"
"\\set use_roles true\n"
"\\endif\n");

/* If the --create option is specified, the target database will be created and connected to.
* The current connection is to another database and we don't want to disable auto analyze on
* that.
*
* TODO: If --create is specified, disable auto analyze after the target database is created
* and we connect to it.
*/
if (!AH->public.ropt->createDB)
{
ahprintf(AH,
"\n-- YB: disable auto analyze to avoid conflicts with catalog changes\n");
ahprintf(AH, yb_disable_auto_analyze_cmd, "on");
}
}

ahprintf(AH, "\n");
Expand Down
16 changes: 16 additions & 0 deletions src/postgres/src/test/regress/data/yb_ysql_dump.data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ SET row_security = off;
\set use_roles true
\endif

-- YB: disable auto analyze to avoid conflicts with catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO on', current_database());
END IF;
END $$;

--
-- Name: hint_plan; Type: SCHEMA; Schema: -; Owner: yugabyte_test
--
Expand Down Expand Up @@ -2488,6 +2496,14 @@ GRANT ALL ON TABLE public.rls_public TO PUBLIC;
\endif


-- YB: re-enable auto analyze after all catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO off', current_database());
END IF;
END $$;

--
-- YSQL database dump complete
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ SET row_security = off;
\set use_roles true
\endif

-- YB: disable auto analyze to avoid conflicts with catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO on', current_database());
END IF;
END $$;

\if :use_tablespaces
SET default_tablespace = '';
\endif
Expand Down Expand Up @@ -485,6 +493,14 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
\endif


-- YB: re-enable auto analyze after all catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO off', current_database());
END IF;
END $$;

--
-- YSQL database dump complete
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ SET row_security = off;
\set use_roles true
\endif

-- YB: disable auto analyze to avoid conflicts with catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO on', current_database());
END IF;
END $$;

SET default_table_access_method = heap;

--
Expand Down Expand Up @@ -444,6 +452,14 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
REFRESH MATERIALIZED VIEW public.mv1;


-- YB: re-enable auto analyze after all catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO off', current_database());
END IF;
END $$;

--
-- YSQL database dump complete
--
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ SET row_security = off;
\set use_roles true
\endif

-- YB: disable auto analyze to avoid conflicts with catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO on', current_database());
END IF;
END $$;

\if :use_tablespaces
SET default_tablespace = '';
\endif
Expand Down Expand Up @@ -431,6 +439,14 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
\endif


-- YB: re-enable auto analyze after all catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO off', current_database());
END IF;
END $$;

--
-- YSQL database dump complete
--
Expand Down
32 changes: 32 additions & 0 deletions src/postgres/src/test/regress/data/yb_ysql_dumpall.data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,14 @@ SET row_security = off;
\set use_roles true
\endif

-- YB: disable auto analyze to avoid conflicts with catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO on', current_database());
END IF;
END $$;

--
-- Name: FUNCTION pg_stat_statements_reset(userid oid, dbid oid, queryid bigint); Type: ACL; Schema: pg_catalog; Owner: postgres
--
Expand Down Expand Up @@ -213,6 +221,14 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
\endif


-- YB: re-enable auto analyze after all catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO off', current_database());
END IF;
END $$;

--
-- YSQL database dump complete
--
Expand Down Expand Up @@ -256,6 +272,14 @@ SET row_security = off;
\set use_roles true
\endif

-- YB: disable auto analyze to avoid conflicts with catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO on', current_database());
END IF;
END $$;

--
-- Name: FUNCTION pg_stat_statements_reset(userid oid, dbid oid, queryid bigint); Type: ACL; Schema: pg_catalog; Owner: postgres
--
Expand Down Expand Up @@ -289,6 +313,14 @@ SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);
\endif


-- YB: re-enable auto analyze after all catalog changes
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM pg_settings WHERE name = 'yb_disable_auto_analyze') THEN
EXECUTE format('ALTER DATABASE %I SET yb_disable_auto_analyze TO off', current_database());
END IF;
END $$;

--
-- YSQL database dump complete
--
Expand Down
2 changes: 1 addition & 1 deletion src/yb/integration-tests/cdc_service-int-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ CDCServiceImpl* CDCService(tserver::TabletServer* tserver) {
class CDCServiceTest : public YBMiniClusterTestBase<MiniCluster> {
protected:
void SetUp() override {
CHECK_OK(SET_FLAG(vmodule, "cdc*=4"));
google::SetVLOGLevel("cdc*", 4);
YBMiniClusterTestBase::SetUp();

MiniClusterOptions opts;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class StatefulServiceTest : public MiniClusterTestWithClient<MiniCluster> {
public:
void SetUp() override {
ANNOTATE_UNPROTECTED_WRITE(FLAGS_TEST_echo_service_enabled) = true;
ASSERT_OK(SET_FLAG(vmodule, "stateful_service*=4"));
google::SetVLOGLevel("stateful_service*", 4);
YBMiniClusterTestBase::SetUp();
MiniClusterOptions opts;
opts.num_tablet_servers = kNumTServers;
Expand Down
1 change: 1 addition & 0 deletions src/yb/server/server_common_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "yb/util/flag_validators.h"
#include "yb/util/flags.h"
#include "yb/util/flag_validators.h"

// User specified identifier for this cluster. On the first master leader setup, this is stored in
// the cluster_config. if not specified, a random UUID is generated.
Expand Down
64 changes: 64 additions & 0 deletions src/yb/tools/yb-backup/yb-backup-cross-feature-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3386,5 +3386,69 @@ TEST_F(
(6 rows)
)#"));
}

class YBBackupTestAutoAnalyze : public YBBackupTest {
public:
void UpdateMiniClusterOptions(ExternalMiniClusterOptions* options) override {
YBBackupTest::UpdateMiniClusterOptions(options);
options->extra_master_flags.push_back("--ysql_enable_auto_analyze_service=true");

options->extra_tserver_flags.push_back("--ysql_enable_auto_analyze_service=true");
options->extra_tserver_flags.push_back("--ysql_enable_table_mutation_counter=true");
options->extra_tserver_flags.push_back("--ysql_node_level_mutation_reporting_interval_ms=10");
options->extra_tserver_flags.push_back("--ysql_cluster_level_mutation_persist_interval_ms=10");
}
};

TEST_F_EX(
YBBackupTest, YB_DISABLE_TEST_IN_SANITIZERS(TestAutoAnalyzeEnabledDuringRestore),
YBBackupTestAutoAnalyze) {
ASSERT_OK(cluster_->SetFlagOnTServers("vmodule", "pg_auto_analyze_service=5"));
const int num_tables = 20;
for (int i = 0; i < num_tables; ++i) {
ASSERT_NO_FATALS(CreateTable(Format("CREATE TABLE tbl_$0(a INT)", i)));
ASSERT_NO_FATALS(InsertRows(Format("INSERT INTO tbl_$0 VALUES (1), (2), (3)", i), 3));
}

// Set auto analyze threshold to a small number to make auto analyze service
// run ANALYZEs aggressively.
ASSERT_OK(cluster_->SetFlagOnTServers("ysql_auto_analyze_threshold", "1"));
ASSERT_OK(cluster_->SetFlagOnTServers("ysql_auto_analyze_scale_factor", "0.1"));
SleepFor(3s * kTimeMultiplier);

// Verify that the auto analyze service is running.
ASSERT_NO_FATALS(RunPsqlCommand(
"SELECT reltuples FROM pg_class WHERE relname = 'tbl_0'",
R"#(
reltuples
-----------
3
(1 row)
)#"
));

// Backup and restore to a new database.
const string backup_dir = GetTempDir("backup");
ASSERT_OK(RunBackupCommand(
{"--backup_location", backup_dir, "--keyspace", "ysql.yugabyte", "create"}));
ASSERT_OK(RunBackupCommand(
{"--backup_location", backup_dir, "--keyspace", "ysql.db2", "restore"}));

// Verify that the auto analyze service is still enabled and works correctly after restore.
SetDbName("db2");
ASSERT_NO_FATALS(CreateTable(Format("CREATE TABLE tbl_$0(a INT)", num_tables)));
ASSERT_NO_FATALS(InsertRows(Format("INSERT INTO tbl_$0 VALUES (1), (2), (3)", num_tables), 3));
SleepFor(3s * kTimeMultiplier);
ASSERT_NO_FATALS(RunPsqlCommand(
Format("SELECT reltuples FROM pg_class WHERE relname = 'tbl_$0'", num_tables),
R"#(
reltuples
-----------
3
(1 row)
)#"
));
}

} // namespace tools
} // namespace yb
Loading

0 comments on commit a842915

Please sign in to comment.