Skip to content

Commit

Permalink
[BACKPORT 2024.1.3][#24050] docdb: Fix re-packing rows after alter ta…
Browse files Browse the repository at this point in the history
…ble 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: 06472d5 / 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
  • Loading branch information
Huqicheng committed Nov 10, 2024
1 parent 7adc21c commit 97872bf
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/yb/docdb/docdb_compaction_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down
9 changes: 5 additions & 4 deletions src/yb/dockv/packed_row.cc
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,8 @@ void RowPackerV1::Init(SchemaVersion version) {
result_.Truncate(prefix_end_);
}

Result<bool> RowPackerV1::AddValue(ColumnId column_id, const QLValuePB& value) {
return DoAddValue(column_id, value, /* tail_size= */ 0);
Result<bool> RowPackerV1::AddValue(ColumnId column_id, const QLValuePB& value, ssize_t tail_size) {
return DoAddValue(column_id, value, tail_size);
}

Result<bool> RowPackerV1::AddValue(ColumnId column_id, const LWQLValuePB& value) {
Expand Down Expand Up @@ -519,8 +519,9 @@ Result<bool> RowPackerV2::AddValue(
return DoAddValue(column_id, std::pair(value_prefix, value_suffix), tail_size);
}

Result<bool> RowPackerV2::AddValue(ColumnId column_id, const QLValuePB& value) {
return DoAddValue(column_id, value, /* tail_size= */ 0);
Result<bool> RowPackerV2::AddValue(
ColumnId column_id, const QLValuePB& value, ssize_t tail_size) {
return DoAddValue(column_id, value, tail_size);
}

Result<bool> RowPackerV2::AddValue(ColumnId column_id, const LWQLValuePB& value) {
Expand Down
4 changes: 2 additions & 2 deletions src/yb/dockv/packed_row.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class RowPackerV1 : public RowPackerBase {
// Add value consisting of 2 parts - value_prefix+value_suffix.
Result<bool> AddValue(
ColumnId column_id, Slice value_prefix, PackedValueV1 value_suffix, ssize_t tail_size);
Result<bool> AddValue(ColumnId column_id, const QLValuePB& value);
Result<bool> AddValue(ColumnId column_id, const QLValuePB& value, ssize_t tail_size = 0);
Result<bool> AddValue(ColumnId column_id, const LWQLValuePB& value);
Result<bool> AddValue(ColumnId column_id, Slice control_fields, const QLValuePB& value);
Result<bool> AddValue(ColumnId column_id, const PackableValue& value);
Expand Down Expand Up @@ -207,7 +207,7 @@ class RowPackerV2 : public RowPackerBase {
// Add value consisting of 2 parts - value_prefix+value_suffix.
Result<bool> AddValue(
ColumnId column_id, Slice value_prefix, PackedValueV1 value_suffix, ssize_t tail_size);
Result<bool> AddValue(ColumnId column_id, const QLValuePB& value);
Result<bool> AddValue(ColumnId column_id, const QLValuePB& value, ssize_t tail_size = 0);
Result<bool> AddValue(ColumnId column_id, const LWQLValuePB& value);
Result<bool> AddValue(ColumnId column_id, const PackableValue& value);

Expand Down
4 changes: 4 additions & 0 deletions src/yb/yql/pgwrapper/pg_packed_row-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 97872bf

Please sign in to comment.