From 292847570492e24e4a7f23d93807ab0175f1e093 Mon Sep 17 00:00:00 2001 From: qhu Date: Tue, 1 Oct 2024 19:20:41 +0000 Subject: [PATCH] [BACKPORT 2.20][#24050] docdb: Fix re-packing rows after alter table add column with default value Summary: Original commit: 06472d50166654d329961d28cef58df01de4a788 / D38242 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 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/D38606 --- 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 c747b11677e1..1a30c3c2ca9b 100644 --- a/src/yb/docdb/docdb_compaction_context.cc +++ b/src/yb/docdb/docdb_compaction_context.cc @@ -375,7 +375,7 @@ class PackedRowData { dockv::PackerBase(&*packer_).schema().GetMissingValueByColumnId(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 89aa7931463c..b4a5f1c0db70 100644 --- a/src/yb/dockv/packed_row.cc +++ b/src/yb/dockv/packed_row.cc @@ -406,8 +406,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) { @@ -489,8 +489,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 d01eecc93002..80652734537a 100644 --- a/src/yb/dockv/packed_row.h +++ b/src/yb/dockv/packed_row.h @@ -160,7 +160,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); @@ -202,7 +202,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 ce8dbd987c1b..52b0ccace667 100644 --- a/src/yb/yql/pgwrapper/pg_packed_row-test.cc +++ b/src/yb/yql/pgwrapper/pg_packed_row-test.cc @@ -685,6 +685,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) {