Skip to content

Commit b0f8213

Browse files
committed
[#26052] xCluster: bump up normal OID counter at start of automatic mode
Summary: In more detail, when xCluster automatic mode replication starts, we now increase the normal space OID counter above any normal space OIDs in use that we might need to preserve across universes as part of xCluster replication. A new function in sys_catalog, ReadHighestNormalPreservableOid, runs on the master leader in the source universe to compute the needed OID to bump to. Did a small amount of tidying/improving. Fixes #26052 Jira: DB-15379 Test Plan: New "unit test" for ReadHighestNormalPreservableOid function: ``` ybd --cxx-test sys_catalog-itest --gtest_filter '*.ReadHighestNormalPreservableOid' ``` Part of this test is currently disabled because the `pg_catalog.binary_upgrade_set_next_heap_relfilenode` directive isn't honored yet; this will be fixed shortly. In the meantime, I temporarily manually changed the Postgres source to honor the directive by editing: ``` ~/code/yugabyte-db/src/postgres/src/backend/catalog/heap.c:1435: /* * YB: The parent partition has DocDB storage, so we preserve * its relfilenode when upgrading. */ if ((RELKIND_HAS_STORAGE(relkind) || (IsYugaByteEnabled() && relkind == RELKIND_PARTITIONED_TABLE)) && !yb_binary_restore) ``` to remove the `&& !yb_binary_restore` part at the end. After doing that, the test passes without disabling anything. New test for checking that the bumping is done correctly end-to-end: ``` ybd --cxx-test xcluster_ddl_replication-test --gtest_filter '*.ReplicationSetUpBumpsOidCounter' ``` I have verified this test fails if I don't bump or if I don't invalidate. Reviewers: xCluster, hsunder Reviewed By: hsunder Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D42692
1 parent 50f7f51 commit b0f8213

11 files changed

+360
-41
lines changed

src/yb/integration-tests/CMakeLists.txt

+2
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ ADD_YB_TEST(load_balancer_multi_table-test)
225225
ADD_YB_TEST(load_balancer_colocated_tables-test)
226226
ADD_YB_TEST(load_balancer_respect_affinity-test)
227227
ADD_YB_TEST(load_balancer_placement_policy-test)
228+
ADD_YB_TEST(sys_catalog-itest)
229+
YB_TEST_TARGET_LINK_LIBRARIES(sys_catalog-itest pg_wrapper_test_base)
228230
ADD_YB_TEST(sys_catalog_respect_affinity-test)
229231
ADD_YB_TEST(restart-test)
230232
ADD_YB_TEST(yb-ts-cli-itest)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
// Copyright (c) YugabyteDB, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
4+
// in compliance with the License. You may obtain a copy of the License at
5+
//
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
//
8+
// Unless required by applicable law or agreed to in writing, software distributed under the License
9+
// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
10+
// or implied. See the License for the specific language governing permissions and limitations
11+
// under the License.
12+
13+
#include <regex>
14+
#include <string>
15+
16+
#include <gtest/gtest.h>
17+
18+
#include "yb/master/catalog_manager_if.h"
19+
#include "yb/master/master_ddl.pb.h"
20+
#include "yb/master/sys_catalog.h"
21+
22+
#include "yb/yql/pgwrapper/pg_mini_test_base.h"
23+
24+
namespace yb::master {
25+
26+
class SysCatalogITest : public pgwrapper::PgMiniTestBase {
27+
public:
28+
void SetUp() override {
29+
TEST_SETUP_SUPER(pgwrapper::PgMiniTestBase);
30+
31+
master::GetNamespaceInfoResponsePB resp;
32+
ASSERT_OK(client_->GetNamespaceInfo(
33+
/*namespace_id=*/std::string(), namespace_name, YQL_DATABASE_PGSQL, &resp));
34+
namespace_id_ = resp.namespace_().id();
35+
36+
google::SetVLOGLevel("catalog_manager*", 1);
37+
}
38+
39+
const std::string namespace_name = "yugabyte";
40+
NamespaceId namespace_id_;
41+
};
42+
43+
TEST_F(SysCatalogITest, ReadHighestNormalPreservableOid) {
44+
auto conn = ASSERT_RESULT(ConnectToDB(namespace_name));
45+
auto database_oid = ASSERT_RESULT(GetPgsqlDatabaseOid(namespace_id_));
46+
auto sys_catalog = ASSERT_RESULT(catalog_manager())->sys_catalog();
47+
48+
auto original_highest_oid =
49+
ASSERT_RESULT(sys_catalog->ReadHighestNormalPreservableOid(database_oid));
50+
// Make sure all the OIDs we are going to use are higher than any of the starting OIDs.
51+
ASSERT_LT(original_highest_oid, 20'000);
52+
53+
// Create secondary space OIDs of each of the kinds we preserve.
54+
{
55+
// Here @ will be replaced by the lowest secondary space OID.
56+
std::string command = R"(
57+
SET yb_binary_restore = true;
58+
SET yb_ignore_pg_class_oids = false;
59+
SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid((@)::pg_catalog.oid);
60+
SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid((@+1)::pg_catalog.oid);
61+
CREATE TYPE high_enum AS ENUM ();
62+
SELECT pg_catalog.binary_upgrade_set_next_pg_enum_oid((@+2)::pg_catalog.oid);
63+
ALTER TYPE high_enum ADD VALUE 'red';
64+
SELECT pg_catalog.binary_upgrade_set_next_heap_pg_class_oid((@+3)::pg_catalog.oid);
65+
SELECT pg_catalog.binary_upgrade_set_next_heap_relfilenode((@+4)::pg_catalog.oid);
66+
CREATE SEQUENCE high_sequence;
67+
)";
68+
ASSERT_OK(conn.Execute(std::regex_replace(
69+
command, std::regex{"@"}, std::to_string(kPgFirstSecondarySpaceObjectId))));
70+
auto oid = ASSERT_RESULT(sys_catalog->ReadHighestNormalPreservableOid(database_oid));
71+
// The addition of these OIDs should not have changed the highest normal space OID at all.
72+
ASSERT_EQ(oid, original_highest_oid);
73+
}
74+
75+
{
76+
// Run CREATE TYPE new_enum AS ENUM ('red', 'orange'); but using 50000-50001 as pg_enum OIDs.
77+
ASSERT_OK(conn.Execute(R"(
78+
SET yb_binary_restore = true;
79+
SET yb_ignore_pg_class_oids = false;
80+
SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('30000'::pg_catalog.oid);
81+
SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('30001'::pg_catalog.oid);
82+
83+
CREATE TYPE new_enum AS ENUM ();
84+
SELECT pg_catalog.binary_upgrade_set_next_pg_enum_oid('50000'::pg_catalog.oid);
85+
ALTER TYPE new_enum ADD VALUE 'red';
86+
SELECT pg_catalog.binary_upgrade_set_next_pg_enum_oid('50001'::pg_catalog.oid);
87+
ALTER TYPE new_enum ADD VALUE 'orange';
88+
)"));
89+
auto oid = ASSERT_RESULT(sys_catalog->ReadHighestNormalPreservableOid(database_oid));
90+
EXPECT_EQ(oid, 50'001);
91+
}
92+
93+
{
94+
// Run CREATE SEQUENCE new_sequence; using pg_class OID 60000.
95+
ASSERT_OK(conn.Execute(R"(
96+
SET yb_binary_restore = true;
97+
SET yb_ignore_pg_class_oids = false;
98+
SELECT pg_catalog.binary_upgrade_set_next_heap_pg_class_oid('60000'::pg_catalog.oid);
99+
SELECT pg_catalog.binary_upgrade_set_next_heap_relfilenode('30010'::pg_catalog.oid);
100+
101+
CREATE SEQUENCE new_sequence;
102+
)"));
103+
auto oid = ASSERT_RESULT(sys_catalog->ReadHighestNormalPreservableOid(database_oid));
104+
EXPECT_EQ(oid, 60000);
105+
}
106+
107+
{
108+
// Run CREATE TYPE new_composite AS (x int); using pg_type OID 70000-70001.
109+
ASSERT_OK(conn.Execute(R"(
110+
SET yb_binary_restore = true;
111+
SET yb_ignore_pg_class_oids = false;
112+
SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('70000'::pg_catalog.oid);
113+
SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('70001'::pg_catalog.oid);
114+
SELECT pg_catalog.binary_upgrade_set_next_heap_pg_class_oid('30020'::pg_catalog.oid);
115+
116+
CREATE TYPE new_composite AS (x int);
117+
)"));
118+
auto oid = ASSERT_RESULT(sys_catalog->ReadHighestNormalPreservableOid(database_oid));
119+
EXPECT_EQ(oid, 70'001);
120+
}
121+
122+
{
123+
// Run CREATE TABLE public.my_table (x integer); attempting to use pg_relfilenode 80000.
124+
ASSERT_OK(conn.Execute(R"(
125+
SET yb_binary_restore = true;
126+
SET yb_ignore_pg_class_oids = false;
127+
SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('30030'::pg_catalog.oid);
128+
SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('30031'::pg_catalog.oid);
129+
SELECT pg_catalog.binary_upgrade_set_next_heap_pg_class_oid('30032'::pg_catalog.oid);
130+
SELECT pg_catalog.binary_upgrade_set_next_heap_relfilenode('80000'::pg_catalog.oid);
131+
132+
CREATE TABLE my_table (x integer);
133+
)"));
134+
135+
// TODO(yhaddad): fix Postgres to honor binary_upgrade_set_next_heap_relfilenode directive then
136+
// update this test to expect 80'000.
137+
//
138+
// (Currently this directive is not honored, which prevents this test from testing the
139+
// relfilenode case.)
140+
LOG(INFO) << ASSERT_RESULT(conn.FetchAllAsString("SELECT oid, relfilenode FROM pg_class;"));
141+
auto oid = ASSERT_RESULT(sys_catalog->ReadHighestNormalPreservableOid(database_oid));
142+
// EXPECT_EQ(oid, 80'000);
143+
EXPECT_EQ(oid, 70'001);
144+
}
145+
}
146+
147+
} // namespace yb::master

src/yb/integration-tests/xcluster/xcluster_ddl_replication-test.cc

+74
Original file line numberDiff line numberDiff line change
@@ -1318,6 +1318,80 @@ TEST_F(XClusterDDLReplicationSwitchoverTest, SwitchoverBumpsAboveUsedOids) {
13181318
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
13191319
}
13201320

1321+
using XClusterDDLReplicationSetupTest = XClusterDDLReplicationSwitchoverTest;
1322+
1323+
TEST_F(XClusterDDLReplicationSetupTest, ReplicationSetUpBumpsOidCounter) {
1324+
if (!UseYbController()) {
1325+
GTEST_SKIP() << "This test does not work with yb_backup.py";
1326+
}
1327+
1328+
// The resulting statement will consume 100 pg_enum OIDs.
1329+
auto CreateGiantEnumStatement = [](std::string name) {
1330+
std::string result = Format("CREATE TYPE $0 AS ENUM ('l0'", name);
1331+
for (int i = 1; i < 100; i++) {
1332+
result += Format(", 'l$0'", i);
1333+
}
1334+
return result + ");";
1335+
};
1336+
1337+
google::SetVLOGLevel("catalog_manager*", 1);
1338+
google::SetVLOGLevel("pg_client_service*", 1);
1339+
google::SetVLOGLevel("xcluster_source_manager", 1);
1340+
// Cache only 30 OIDs at a time; see below for why this value was chosen.
1341+
ANNOTATE_UNPROTECTED_WRITE(FLAGS_ysql_oid_cache_prefetch_size) = 30;
1342+
1343+
ASSERT_OK(SetUpClusters(/*is_colocated=*/false, /*start_yb_controller_servers=*/true));
1344+
1345+
// Create a giant enum on cluster A.
1346+
{
1347+
auto conn = ASSERT_RESULT(cluster_A_->ConnectToDB(namespace_name));
1348+
ASSERT_OK(conn.Execute(CreateGiantEnumStatement("first_enum")));
1349+
// Backup requires at least one table so create one.
1350+
ASSERT_OK(conn.Execute("CREATE TABLE my_table (x INT);"));
1351+
}
1352+
1353+
// Reset OID counters on A by backing up then restoring the database on cluster A.
1354+
ASSERT_OK(BackupFromProducer());
1355+
SetReplicationDirection(ReplicationDirection::BToA);
1356+
ASSERT_OK(RestoreToConsumer());
1357+
SetReplicationDirection(ReplicationDirection::AToB);
1358+
1359+
// Allocate a few OIDs on A to force caching of normal space OIDs.
1360+
{
1361+
auto conn = ASSERT_RESULT(cluster_A_->ConnectToDB(namespace_name));
1362+
ASSERT_OK(conn.Execute("CREATE TABLE exercise_cache (x INT);"));
1363+
}
1364+
1365+
// Set up xCluster replication with a nonempty database.
1366+
ASSERT_OK(CheckpointReplicationGroupOnNamespaces({namespace_name}));
1367+
ASSERT_OK(BackupFromProducer());
1368+
ASSERT_OK(RestoreToConsumer());
1369+
ASSERT_OK(CreateReplicationFromCheckpoint());
1370+
1371+
// At this point, in the absence of OID cache invalidation, we would still have OIDs cached on A
1372+
// from before replication was set up. (Replication setup does create some objects, consuming
1373+
// OIDs, but not enough to exhaust the cache size of 30 we have set.)
1374+
//
1375+
// Importantly, we do not have enough OIDs cached to handle an entire giant enum.
1376+
1377+
{
1378+
// Drop via manual DDL replication the enum on only cluster A.
1379+
auto conn = ASSERT_RESULT(cluster_A_->ConnectToDB(namespace_name));
1380+
ASSERT_OK(conn.Execute(R"(
1381+
SET yb_xcluster_ddl_replication.enable_manual_ddl_replication=1;
1382+
DROP TYPE first_enum;
1383+
SET yb_xcluster_ddl_replication.enable_manual_ddl_replication=0;)"));
1384+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
1385+
1386+
// Now create a new giant enum via normal DDL replication. If we did not bump up the normal
1387+
// space OID counter and invalidate on A then this will cause a OID collision on cluster B
1388+
// because the new enum on A will use OIDs freed up by dropping the previous enum but those OIDs
1389+
// are still in use on B because the previous enum still exists there.
1390+
ASSERT_OK(conn.Execute(CreateGiantEnumStatement("second_enum")));
1391+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
1392+
}
1393+
}
1394+
13211395
class XClusterDDLReplicationAddDropColumnTest : public XClusterDDLReplicationTest {
13221396
public:
13231397
void SetUp() override {

0 commit comments

Comments
 (0)