Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[xCluster] Setup does not work with tables with the same names but in different schemas #11691

Closed
hulien22 opened this issue Mar 8, 2022 · 1 comment
Closed

[xCluster] Setup does not work with tables with the same names but in different schemas #11691

hulien22 opened this issue Mar 8, 2022 · 1 comment
Assignees
@bmatican
Labels
area/docdb YugabyteDB core features kind/bug This issue is a bug priority/medium Medium priority issue xCluster Label for xCluster related issues/improvements

Comments

@hulien22
Copy link
Contributor

hulien22 commented Mar 8, 2022 •
edited by yugabyte-ci
Loading

Jira Link: DB-806

Description

Perform on both source and target:

create schema test1;
create schema test2;
create table test1.example(i int primary key, v text);
create table test2.example(i int primary key, v text);

Try to setup replication on test2.example, and we'll get an error of the form:

Error waiting for universe replication setup to complete: Source and target schemas don't match: Source: 00004000000030008000000000004007, Target: 0000400000003000800000000000400d, Source schema: Schema [
<...>

This happens since we currently just match tables on the target side by looking for a table with the same dbname and same table name. We need to also check for schema name here.
@hulien22 hulien22 added area/docdb YugabyteDB core features xCluster Label for xCluster related issues/improvements labels Mar 8, 2022
@hulien22
Copy link
Contributor Author

hulien22 commented Mar 8, 2022

Note: CatalogManager::ValidateTableSchema seems to have a lot of comments about doing a temporary approach to get target side tables, but since it looks like the necessary issues have been closed, perhaps we can cleanup that code as well..

yugabyte-db/ent/src/yb/master/catalog_manager_ent.cc

Lines 3231 to 3233 in 098fd69

// Since YSQL tables are not present in table map, we first need to list tables to get the table
// ID and then get table schema.
// Remove this once table maps are fixed for YSQL.

yugabyte-db/ent/src/yb/master/catalog_manager_ent.cc

Lines 3243 to 3244 in 098fd69

// TODO: This does not work for situation where tables in different YSQL schemas have the same
// name. This will be fixed as part of #1476.

@bmatican bmatican mentioned this issue Mar 23, 2022
Closed
LambdaWL added a commit that referenced this issue May 31, 2022
@LambdaWL
[#11691] xCluster: Also check for schema name match when setup xclust…
9737277 
…er replication

Summary:
#11691

Resolved the previous issue where if tables test1.example, test2.example are created on both producer and consumer, a replication setup for either of these two tables will fail.

Added unit test.

Test Plan:
Verify locally that the problem described in the Github issue no longer exists. Verify that the unit test runs without error:
```
./yb_build.sh --cxx-test integration-tests_twodc_ysql-test --gtest_filter "*SetupSameNameDifferentSchemaUniverseReplication/*" -n 4
```

Also did some manual testings:
- Ensured that without the change, the above unit test failed
- Ensured that the backfill succeeded by manually removing the check condition under CatalogManager::GetTableSchemaInternal

Reviewers: jhe

Reviewed By: jhe

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D17099
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jun 8, 2022
LambdaWL added a commit that referenced this issue Jun 10, 2022
@LambdaWL
[#11691] xCluster: (Fix) Do not return error when schema name not fou…
3427342 
…nd for YSQL tables

Summary:
This is a fix for an issue introduced by the previous  diff https://phabricator.dev.yugabyte.com/D17099

Issue: https://yugabyte.slack.com/archives/C03G3F2H4AU/p1654647033631909

It appears that it is possible that some YSQL tables' schema names can not be found even in pg system tables. Previously, an error was raised in this case. The fix is to change the error to a warning message.

Test Plan:
Manually let all non-system YSQL tables passing the two if-clauses in the following block in `CatalogManager::GetTableSchemaInternal`:
```
if (!table->is_system() && l->table_type() == TableType::PGSQL_TABLE_TYPE &&
      !resp->schema().has_pgschema_name()) {
    SharedLock lock(mutex_);
    TRACE("Acquired catalog manager lock for schema name lookup");

    auto pgschema_name = GetPgSchemaName(table);
    if (!pgschema_name.ok() || pgschema_name->empty()) {
      LOG(WARNING) << Format(
          "Unable to find schema name for YSQL table $0.$1 due to error: $2",
          table->namespace_name(), table->name(), pgschema_name.ToString());
    } else {
      resp->mutable_schema()->set_pgschema_name(*pgschema_name);
    }
  }
```
Verify that no error is returned.

Reviewers: jhe

Reviewed By: jhe

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D17563
LambdaWL added a commit that referenced this issue Jun 15, 2022
@LambdaWL
[BACKPORT 2.15.0][#11691] xCluster: (Fix) Do not return error when sc…
c0584f9 
…hema name not found for YSQL tables

Summary:
Original commit: 3427342 / D17563

This is a fix for an issue introduced by the previous  diff https://phabricator.dev.yugabyte.com/D17099

Issue: https://yugabyte.slack.com/archives/C03G3F2H4AU/p1654647033631909

It appears that it is possible that some YSQL tables' schema names can not be found even in pg system tables. Previously, an error was raised in this case. The fix is to change the error to a warning message.

Test Plan:
Manually let all non-system YSQL tables passing the two if-clauses in the following block in `CatalogManager::GetTableSchemaInternal`:
```
if (!table->is_system() && l->table_type() == TableType::PGSQL_TABLE_TYPE &&
      !resp->schema().has_pgschema_name()) {
    SharedLock lock(mutex_);
    TRACE("Acquired catalog manager lock for schema name lookup");

    auto pgschema_name = GetPgSchemaName(table);
    if (!pgschema_name.ok() || pgschema_name->empty()) {
      LOG(WARNING) << Format(
          "Unable to find schema name for YSQL table $0.$1 due to error: $2",
          table->namespace_name(), table->name(), pgschema_name.ToString());
    } else {
      resp->mutable_schema()->set_pgschema_name(*pgschema_name);
    }
  }
```
Verify that no error is returned.

Reviewers: jhe

Reviewed By: jhe

Subscribers: bogdan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D17656
hulien22 added a commit that referenced this issue Sep 28, 2022
@hulien22
[BACKPORT 2.14][#11691] xCluster: Also check for schema name match wh…
32bd63f 
…en setup xcluster replication

Summary:
Original commit: 9737277 / D17099
#11691

Resolved the previous issue where if tables test1.example, test2.example are created on both producer and consumer, a replication setup for either of these two tables will fail.

Added unit test.

Test Plan:
Verify locally that the problem described in the Github issue no longer exists. Verify that the unit test runs without error:
```
./yb_build.sh --cxx-test integration-tests_twodc_ysql-test --gtest_filter "*SetupSameNameDifferentSchemaUniverseReplication/*" -n 4
```

Also did some manual testings:
- Ensured that without the change, the above unit test failed
- Ensured that the backfill succeeded by manually removing the check condition under CatalogManager::GetTableSchemaInternal

Reviewers: slingam

Reviewed By: slingam

Subscribers: bogdan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D19609
hulien22 added a commit that referenced this issue Dec 16, 2022
@hulien22
[BACKPORT 2.14][#11691] xCluster: (Fix) Do not return error when sche…
c5c69b3 
…ma name not found for YSQL tables

Summary:
Original commit: 3427342 / D17563
This is a fix for an issue introduced by the previous  diff https://phabricator.dev.yugabyte.com/D17099

Issue: https://yugabyte.slack.com/archives/C03G3F2H4AU/p1654647033631909

It appears that it is possible that some YSQL tables' schema names can not be found even in pg system tables. Previously, an error was raised in this case. The fix is to change the error to a warning message.

Test Plan:
Manually let all non-system YSQL tables passing the two if-clauses in the following block in `CatalogManager::GetTableSchemaInternal`:
```
if (!table->is_system() && l->table_type() == TableType::PGSQL_TABLE_TYPE &&
      !resp->schema().has_pgschema_name()) {
    SharedLock lock(mutex_);
    TRACE("Acquired catalog manager lock for schema name lookup");

    auto pgschema_name = GetPgSchemaName(table);
    if (!pgschema_name.ok() || pgschema_name->empty()) {
      LOG(WARNING) << Format(
          "Unable to find schema name for YSQL table $0.$1 due to error: $2",
          table->namespace_name(), table->name(), pgschema_name.ToString());
    } else {
      resp->mutable_schema()->set_pgschema_name(*pgschema_name);
    }
  }
```
Verify that no error is returned.

Reviewers: nicolas, rahuldesirazu

Reviewed By: rahuldesirazu

Subscribers: yguan, bogdan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D21789
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bmatican bmatican

Labels
area/docdb YugabyteDB core features kind/bug This issue is a bug priority/medium Medium priority issue xCluster Label for xCluster related issues/improvements
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bmatican @yugabyte-ci @hulien22