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

[DocDB] Packed: Restore should update table schema_version to the one in the backup #14296

Closed
bmatican opened this issue Oct 3, 2022 · 1 comment
Assignees
Labels
2.16.0_blocker 2.16.0 Release blocker defects area/docdb YugabyteDB core features kind/bug This issue is a bug priority/high High Priority

Comments

@bmatican
Copy link
Contributor

bmatican commented Oct 3, 2022

Jira Link: DB-3735

Description

For packed, we keep a reference to the older schemas, by versioning them in a SchemaPacking version on the tserver + keeping a schema_version ID in the row itself.

However, on restore, when we create tables, we just leave them at their default schema_version=0, without updating to the correct version from the backup. This breaks packed, if the data has any extra schemas, ie: if any Alters were run on the original data...

Master
On restore, we need to modify CatalogManager::ImportTableEntry, to also patch the schema version and send the corresponding Alters to the tservers.

Tserver
Similar to aef944a, we need to patch the tablet metadata to pick up the SchemaPacking from the backup as well.

Note: Since we caught this via manual testing, this suggests we do not have an in-repo test that does this workload + alter + backup/restore. We should make sure we add this, as a regression test, as part of this work!

cc @spolitov @OlegLoginov @rthallamko3

@bmatican bmatican added area/docdb YugabyteDB core features priority/high High Priority status/awaiting-triage Issue awaiting triage labels Oct 3, 2022
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug and removed status/awaiting-triage Issue awaiting triage labels Oct 3, 2022
@kripasreenivasan kripasreenivasan added the 2.16.0_blocker 2.16.0 Release blocker defects label Oct 13, 2022
sanketkedia added a commit that referenced this issue Nov 1, 2022
…on restore

Summary:
For packed, we keep a reference to the older schemas, by versioning them in a SchemaPacking version
on the tserver + keeping a schema_version ID in the row itself.

However, on restore, when we create tables, we just leave them at their default schema_version of 1,
without updating to the correct version from the backup and without merging all the schema packings
contained in the snapshot. This breaks packed, if the data has any
extra schemas, ie: if any Alters were run on the original data.

This diff merges all the schema packings present in the snapshot with schema packings already
present for the table in the restoration. It also bumps up the schema version to the version
of the snapshot during import snapshot.

For e.g. if snapshot has:
0 - packing0
1 - packing1
2 - packing2

and restore has:
0 - newPacking0
1 - newPacking1

The merge will produce:
0 - packing0
1 - packing1
2 - packing2
and the schema version will be bumped up to 2.

If snapshot has:
0 - packing0

and restore has:
0 - newPacking0
1 - newPacking1

merge will produce:
0 - packing0
1 - newPacking1
with schema version as 1

Also, note that this only fixes the non-colocated case. Changes for the colocated case are more
involved and will be part of another diff.

Test Plan:
./yb_build.sh --cxx_test yb-backup-test_ent --gtest-filter YBBackupTestWithPackedRows.YSQLSchemaPackingWithSnapshotGreaterVersionThanRestore
./yb_build.sh --cxx_test yb-backup-test_ent --gtest-filter YBBackupTestWithPackedRows.YSQLSchemaPackingWithSnapshotLowerVersionThanRestore

Reviewers: oleg, zdrudi, slingam, sergei

Reviewed By: slingam, sergei

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D20301
@rthallamko3 rthallamko3 reopened this Nov 1, 2022
@rthallamko3
Copy link
Contributor

@sanketkedia , reopening for backports to 2.16 and 2.17.0 branches.

sanketkedia added a commit that referenced this issue Nov 3, 2022
…acking history on restore

Summary:
Original commit: d04f8ee / D20301
For packed, we keep a reference to the older schemas, by versioning them in a SchemaPacking version
on the tserver + keeping a schema_version ID in the row itself.

However, on restore, when we create tables, we just leave them at their default schema_version of 1,
without updating to the correct version from the backup and without merging all the schema packings
contained in the snapshot. This breaks packed, if the data has any
extra schemas, ie: if any Alters were run on the original data.

This diff merges all the schema packings present in the snapshot with schema packings already
present for the table in the restoration. It also bumps up the schema version to the version
of the snapshot during import snapshot.

For e.g. if snapshot has:
0 - packing0
1 - packing1
2 - packing2

and restore has:
0 - newPacking0
1 - newPacking1

The merge will produce:
0 - packing0
1 - packing1
2 - packing2
and the schema version will be bumped up to 2.

If snapshot has:
0 - packing0

and restore has:
0 - newPacking0
1 - newPacking1

merge will produce:
0 - packing0
1 - newPacking1
with schema version as 1

Also, note that this only fixes the non-colocated case. Changes for the colocated case are more
involved and will be part of another diff.

Test Plan:
./yb_build.sh --cxx_test yb-backup-test_ent --gtest-filter YBBackupTestWithPackedRows.YSQLSchemaPackingWithSnapshotGreaterVersionThanRestore
./yb_build.sh --cxx_test yb-backup-test_ent --gtest-filter YBBackupTestWithPackedRows.YSQLSchemaPackingWithSnapshotLowerVersionThanRestore

Reviewers: oleg, slingam, sergei, zdrudi

Reviewed By: zdrudi

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D20725
sanketkedia added a commit that referenced this issue Nov 4, 2022
… packing history on restore

Summary:
Original commit: d04f8ee / D20301
For packed, we keep a reference to the older schemas, by versioning them in a SchemaPacking version
on the tserver + keeping a schema_version ID in the row itself.

However, on restore, when we create tables, we just leave them at their default schema_version of 1,
without updating to the correct version from the backup and without merging all the schema packings
contained in the snapshot. This breaks packed, if the data has any
extra schemas, ie: if any Alters were run on the original data.

This diff merges all the schema packings present in the snapshot with schema packings already
present for the table in the restoration. It also bumps up the schema version to the version
of the snapshot during import snapshot.

For e.g. if snapshot has:
0 - packing0
1 - packing1
2 - packing2

and restore has:
0 - newPacking0
1 - newPacking1

The merge will produce:
0 - packing0
1 - packing1
2 - packing2
and the schema version will be bumped up to 2.

If snapshot has:
0 - packing0

and restore has:
0 - newPacking0
1 - newPacking1

merge will produce:
0 - packing0
1 - newPacking1
with schema version as 1

Also, note that this only fixes the non-colocated case. Changes for the colocated case are more
involved and will be part of another diff.

Test Plan:
./yb_build.sh --cxx_test yb-backup-test_ent --gtest-filter YBBackupTestWithPackedRows.YSQLSchemaPackingWithSnapshotGreaterVersionThanRestore
./yb_build.sh --cxx_test yb-backup-test_ent --gtest-filter YBBackupTestWithPackedRows.YSQLSchemaPackingWithSnapshotLowerVersionThanRestore

Reviewers: oleg, slingam, sergei, zdrudi

Reviewed By: zdrudi

Subscribers: bogdan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D20722
jayant07-yb pushed a commit to jayant07-yb/yugabyte-db that referenced this issue Dec 7, 2022
…history on restore

Summary:
For packed, we keep a reference to the older schemas, by versioning them in a SchemaPacking version
on the tserver + keeping a schema_version ID in the row itself.

However, on restore, when we create tables, we just leave them at their default schema_version of 1,
without updating to the correct version from the backup and without merging all the schema packings
contained in the snapshot. This breaks packed, if the data has any
extra schemas, ie: if any Alters were run on the original data.

This diff merges all the schema packings present in the snapshot with schema packings already
present for the table in the restoration. It also bumps up the schema version to the version
of the snapshot during import snapshot.

For e.g. if snapshot has:
0 - packing0
1 - packing1
2 - packing2

and restore has:
0 - newPacking0
1 - newPacking1

The merge will produce:
0 - packing0
1 - packing1
2 - packing2
and the schema version will be bumped up to 2.

If snapshot has:
0 - packing0

and restore has:
0 - newPacking0
1 - newPacking1

merge will produce:
0 - packing0
1 - newPacking1
with schema version as 1

Also, note that this only fixes the non-colocated case. Changes for the colocated case are more
involved and will be part of another diff.

Test Plan:
./yb_build.sh --cxx_test yb-backup-test_ent --gtest-filter YBBackupTestWithPackedRows.YSQLSchemaPackingWithSnapshotGreaterVersionThanRestore
./yb_build.sh --cxx_test yb-backup-test_ent --gtest-filter YBBackupTestWithPackedRows.YSQLSchemaPackingWithSnapshotLowerVersionThanRestore

Reviewers: oleg, zdrudi, slingam, sergei

Reviewed By: slingam, sergei

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D20301
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.16.0_blocker 2.16.0 Release blocker defects area/docdb YugabyteDB core features kind/bug This issue is a bug priority/high High Priority
Projects
None yet
Development

No branches or pull requests

6 participants