From 97872bf3094f0eda9b42e91ced9e00e29999a57f Mon Sep 17 00:00:00 2001 From: qhu Date: Sat, 9 Nov 2024 16:00:50 +0000 Subject: [PATCH] [BACKPORT 2024.1.3][#24050] docdb: Fix re-packing rows after alter table add column with default value Summary: D17904 fixed re-packing failure due to packed row size overflow. There's a similar issue where compaction follows an `alter table` statement that adds a column with default value eg `ALTER TABLE t ADD COLUMN v3 TIMESTAMP DEFAULT CURRENT_TIMESTAMP NULL` Compaction will fail with the error like `Unable to pack old value for 10` Fix this by allowing the packed row size exceeds the size limit during re-packing. Modified PgPackedRowTest.PackOverflow to also test with column with default value. Without this fix, compaction will fail with ``` E0919 21:12:51.649142 920988 db_impl.cc:3417] T 0685c068ecf34c9b8252aa37a12f7de7 P 5e3d933d73d2450180c36a3cf5b3a74b [R]: Waiting after background compaction error: Corruption (yb/docdb/docdb_compaction_context.cc:403): Unable to pack old value for 2, Accumulated background error counts: 1 ../../src/yb/yql/pgwrapper/pg_packed_row-test.cc:712: Failure Failed Bad status: Corruption (yb/docdb/docdb_compaction_context.cc:403): Compact range failed: Unable to pack old value for 2 ``` Jira: DB-12940 Original commit: 06472d50166654d329961d28cef58df01de4a788 / D38242 Test Plan: ./yb_build.sh release --cxx-test pg_packed_row-test --gtest_filter "PackingVersion/PgPackedRowTest.PackOverflow/*" Reviewers: sergei, arybochkin, rthallam Reviewed By: rthallam Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D39856 --- src/yb/docdb/docdb_compaction_context.cc | 2 +- src/yb/dockv/packed_row.cc | 9 +++++---- src/yb/dockv/packed_row.h | 4 ++-- src/yb/yql/pgwrapper/pg_packed_row-test.cc | 4 ++++ 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/yb/docdb/docdb_compaction_context.cc b/src/yb/docdb/docdb_compaction_context.cc index 9127d9ec2006..e764f105844d 100644 --- a/src/yb/docdb/docdb_compaction_context.cc +++ b/src/yb/docdb/docdb_compaction_context.cc @@ -376,7 +376,7 @@ class PackedRowData { column_id)); return CheckPackOldValueResult( column_id, std::visit([column_id, missing_value](auto& packer) { - return packer.AddValue(column_id, missing_value); + return packer.AddValue(column_id, missing_value, kUnlimitedTail); }, *packer_)); } return DoPackOldValue(column_id, decoder.FetchValue(decoder.GetPackedIndex(column_id))); diff --git a/src/yb/dockv/packed_row.cc b/src/yb/dockv/packed_row.cc index 62b9d5ca52de..9732818315a5 100644 --- a/src/yb/dockv/packed_row.cc +++ b/src/yb/dockv/packed_row.cc @@ -432,8 +432,8 @@ void RowPackerV1::Init(SchemaVersion version) { result_.Truncate(prefix_end_); } -Result RowPackerV1::AddValue(ColumnId column_id, const QLValuePB& value) { - return DoAddValue(column_id, value, /* tail_size= */ 0); +Result RowPackerV1::AddValue(ColumnId column_id, const QLValuePB& value, ssize_t tail_size) { + return DoAddValue(column_id, value, tail_size); } Result RowPackerV1::AddValue(ColumnId column_id, const LWQLValuePB& value) { @@ -519,8 +519,9 @@ Result RowPackerV2::AddValue( return DoAddValue(column_id, std::pair(value_prefix, value_suffix), tail_size); } -Result RowPackerV2::AddValue(ColumnId column_id, const QLValuePB& value) { - return DoAddValue(column_id, value, /* tail_size= */ 0); +Result RowPackerV2::AddValue( + ColumnId column_id, const QLValuePB& value, ssize_t tail_size) { + return DoAddValue(column_id, value, tail_size); } Result RowPackerV2::AddValue(ColumnId column_id, const LWQLValuePB& value) { diff --git a/src/yb/dockv/packed_row.h b/src/yb/dockv/packed_row.h index 7f139a773a75..3bb645ef71c0 100644 --- a/src/yb/dockv/packed_row.h +++ b/src/yb/dockv/packed_row.h @@ -164,7 +164,7 @@ class RowPackerV1 : public RowPackerBase { // Add value consisting of 2 parts - value_prefix+value_suffix. Result AddValue( ColumnId column_id, Slice value_prefix, PackedValueV1 value_suffix, ssize_t tail_size); - Result AddValue(ColumnId column_id, const QLValuePB& value); + Result AddValue(ColumnId column_id, const QLValuePB& value, ssize_t tail_size = 0); Result AddValue(ColumnId column_id, const LWQLValuePB& value); Result AddValue(ColumnId column_id, Slice control_fields, const QLValuePB& value); Result AddValue(ColumnId column_id, const PackableValue& value); @@ -207,7 +207,7 @@ class RowPackerV2 : public RowPackerBase { // Add value consisting of 2 parts - value_prefix+value_suffix. Result AddValue( ColumnId column_id, Slice value_prefix, PackedValueV1 value_suffix, ssize_t tail_size); - Result AddValue(ColumnId column_id, const QLValuePB& value); + Result AddValue(ColumnId column_id, const QLValuePB& value, ssize_t tail_size = 0); Result AddValue(ColumnId column_id, const LWQLValuePB& value); Result AddValue(ColumnId column_id, const PackableValue& value); diff --git a/src/yb/yql/pgwrapper/pg_packed_row-test.cc b/src/yb/yql/pgwrapper/pg_packed_row-test.cc index 4b50431594d9..4ec5692eaf58 100644 --- a/src/yb/yql/pgwrapper/pg_packed_row-test.cc +++ b/src/yb/yql/pgwrapper/pg_packed_row-test.cc @@ -690,6 +690,10 @@ TEST_P(PgPackedRowTest, PackOverflow) { ASSERT_OK(conn.Execute("ALTER TABLE t ADD COLUMN v2 TEXT")); ASSERT_OK(cluster_->CompactTablets()); + + ASSERT_OK(conn.Execute("ALTER TABLE t ADD COLUMN v3 TIMESTAMP DEFAULT CURRENT_TIMESTAMP NULL")); + + ASSERT_OK(cluster_->CompactTablets()); } TEST_P(PgPackedRowTest, AddDropColumn) {