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.

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

[YSQL] upgrade issue from 2.0.5.7 to 2.2.2.1 - FATAL: Query error: Bad schema: Cannot have both cotable ID and pgtable ID #5658

Closed
rao-vasireddy opened this issue Sep 11, 2020 · 5 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue wontfix

Comments

@rao-vasireddy
Copy link
Contributor

rao-vasireddy commented Sep 11, 2020

Jira Link: DB-4847
When upgrading a universe running 2.0.5.7 and having YSQL enabled with YSQL tables/indexes present, got an error when connecting to the db using ysqlsh

[yugabyte@ip-10-241-0-146 bin]$ ./ysqlsh -h 10.241.0.146
ysqlsh: FATAL: Query error: Bad schema: Cannot have both cotable ID and pgtable ID

@rao-vasireddy rao-vasireddy added the kind/bug This issue is a bug label Sep 11, 2020
@jaki
Copy link
Contributor

jaki commented Sep 11, 2020

In debug mode, you hit an earlier check:

  void set_cotable_id(const Uuid& cotable_id) {
    if (!cotable_id.IsNil()) {
      DCHECK_EQ(pgtable_id_, 0);
    }
    cotable_id_ = cotable_id;
  }

It seems that the TableInfo has a pgtable_id, but it's trying to add cotable_id. This seems to happen for system tables, so their having a pgtable_id is wrong, even though the number is itself correct.

For those who don't know, system tables use cotable id because all system tables from all databases go in the system catalog tablet on master, and using pgtable id would not be able to distinguish between system tables across databases.

@jaki
Copy link
Contributor

jaki commented Sep 12, 2020

Found it. The issue is with is_ysql_catalog_table.

git log -S is_ysql_catalog_table --oneline
6dfe2f039 colocation: set pgtable_id for SetSchema (#4986)
f16e5caf9 #4233: Check for properties equivalence when comparing schemas for cdc replication
94e2c3ad5 [YSQL] Support alter table for colocated tables: #4293
f4d4ea7d9 #3365: [Colocation] Use 4 byte PG table ID as dockey prefix
b68b84bfd #3108 Limited transactional DDL: each DDL statement in its own transaction

Commit b68b84b introduced is_ysql_catalog_table field in TablePropertiesPB.

  1. TablePropertiesPB is used in SchemaPB
  2. SchemaPB is used in TableInfoPB

Commit f4d4ea7 changes KvStoreInfo::LoadTablesFromPB:

diff --git a/src/yb/tablet/tablet_metadata.cc b/src/yb/tablet/tablet_metadata.cc
index 36d130fa7..e19e2efa8 100644
--- a/src/yb/tablet/tablet_metadata.cc
+++ b/src/yb/tablet/tablet_metadata.cc
@@ -40,6 +40,7 @@
 #include <boost/optional.hpp>
 #include "yb/rocksdb/db.h"
 #include "yb/rocksdb/options.h"
+#include "yb/common/entity_ids.h"
 #include "yb/common/wire_protocol.h"
 #include "yb/consensus/opid_util.h"
 #include "yb/docdb/docdb_rocksdb_util.h"
@@ -171,11 +172,16 @@ Status KvStoreInfo::LoadTablesFromPB(
     auto table_info = std::make_unique<TableInfo>();
     RETURN_NOT_OK(table_info->LoadFromPB(table_pb));
     if (table_info->table_id != primary_table_id) {
-      Uuid cotable_id;
-      CHECK_OK(cotable_id.FromHexString(table_info->table_id));
-      // TODO(#79): when adding for multiple KV-stores per Raft group support - check if we need
-      // to set cotable ID.
-      table_info->schema.set_cotable_id(cotable_id);
+      if (table_pb.schema().table_properties().is_ysql_catalog_table()) {
+        Uuid cotable_id;
+        CHECK_OK(cotable_id.FromHexString(table_info->table_id));
+        // TODO(#79): when adding for multiple KV-stores per Raft group support - check if we need
+        // to set cotable ID.
+        table_info->schema.set_cotable_id(cotable_id);
+      } else {
+        auto pgtable_id = VERIFY_RESULT(GetPgsqlTableOid(table_info->table_id));
+        table_info->schema.set_pgtable_id(pgtable_id);
+      }
     }
     tables[table_info->table_id] = std::move(table_info);
   }

The PBs won't have is_ysql_catalog_table on version 2.0.5.7 because that's before commit b68b84b. Upgrading those clusters to versions including and after commit f4d4ea7 will cause the table infos of postgres system tables to get incorrectly populated with pgtable_id rather than cotable_id because we'll always have is_ysql_catalog_table as false.

To solve this,

  • I saw commit b68b84b had some upgrade handling, so maybe we can use that
  • I saw a MigrateSuperblockForD5900 in tablet_metadata.cc that we could also possibly imitate
  • I think upgrading to v2.0.9 or v2.0.10 as an intermediate step might work since those releases are in between the mentioned commits

@jaki
Copy link
Contributor

jaki commented Feb 26, 2022

Repro:

  1. Get releases:
  2. Create rf 3 node using 2.0.5.2
  3. Rolling upgrade that cluster to 2.2.2.0
  4. Rolling upgrade that cluster to 2.6.11.0 (when new master starts up, it should FATAL within a minute)

Upgrading to debug build can make it fail earlier as mentioned in #5658 (comment). That can be avoided by applying patch in #11229 (comment), but there's still another issue (described in that same comment).

@frozenspider
Copy link
Contributor

Current fix-in-the-works for #7378 does not alter this behaviour.

@rthallamko3 rthallamko3 added the area/ysql Yugabyte SQL (YSQL) label Jan 3, 2023
@yugabyte-ci yugabyte-ci added priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage labels Jan 3, 2023
@m-iancu
Copy link
Contributor

m-iancu commented Jan 22, 2023

Closing as 2.0.5 is very old version, and rolling upgrade from 2.0.9 onwards should work.

@m-iancu m-iancu closed this as completed Jan 22, 2023
@yugabyte-ci yugabyte-ci closed this as not planned Won't fix, can't repro, duplicate, stale Jan 22, 2023
@yugabyte-ci yugabyte-ci removed the status/awaiting-triage Issue awaiting triage label Jan 22, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue wontfix
Projects
Status: Done
Development

No branches or pull requests

6 participants