This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 3666d2026 KUDU-3619 disable KUDU-3367 behavior by default
3666d2026 is described below
commit 3666d2026d48adb5ff636321ef22320a8af5facb
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Tue Sep 24 12:00:39 2024 -0700
KUDU-3619 disable KUDU-3367 behavior by default
As it turned out, KUDU-3367 has introduced a regression due to
a deficiency in its implementation, where major compactions would fail
with errors like below if it had kicked in:
Corruption: Failed major delta compaction on RowSet(1): No min key found:
CFile base data in RowSet(1)
Since KUDU-3367 isn't quite relevant in Kudu versions of 1.12.0 and
newer when working with data that supports live row count (see
KUDU-1625), a quick-and-dirty fix is to set the default value for the
corresponding flag --all_delete_op_delta_file_cnt_for_compaction
to a value that effectively disables KUDU-3367 behavior.
This patch does exactly so.
Change-Id: Iec0719462e379b7a0fb05ca011bb9cdd991a58ef
Reviewed-on: http://gerrit.cloudera.org:8080/21848
Reviewed-by: KeDeng <kdeng...@gmail.com>
Tested-by: Alexey Serbin <ale...@apache.org>
---
src/kudu/tablet/delta_tracker.cc | 12 ++++++------
src/kudu/tablet/diskrowset-test.cc | 12 ++++++++++++
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/kudu/tablet/delta_tracker.cc b/src/kudu/tablet/delta_tracker.cc
index f6a958482..469d6288c 100644
--- a/src/kudu/tablet/delta_tracker.cc
+++ b/src/kudu/tablet/delta_tracker.cc
@@ -18,6 +18,7 @@
#include "kudu/tablet/delta_tracker.h"
#include <algorithm>
+#include <limits>
#include <optional>
#include <ostream>
#include <set>
@@ -61,15 +62,14 @@
DECLARE_int32(tablet_history_max_age_sec);
-DEFINE_uint64(all_delete_op_delta_file_cnt_for_compaction, 1,
+DEFINE_uint64(all_delete_op_delta_file_cnt_for_compaction,
+ std::numeric_limits<uint64_t>::max(),
"The minimum number of REDO delta files containing only ancient
DELETE "
"operations to schedule a major delta compaction on them. A
DELETE "
"operation is considered ancient if it was applied more than "
- "--tablet_history_max_age_sec seconds ago. "
- "If you want to control the number of REDO delta files
containing only "
- "ancient DELETE operations, you can turn up this parameter
value. "
- "Otherwise, it is recommended to keep the default value to
release "
- "storage space efficiently.");
+ "--tablet_history_max_age_sec seconds ago. With the default "
+ "setting, this behavior is effectively disabled; see KUDU-3619 "
+ "for details.");
using kudu::cfile::ReaderOptions;
using kudu::fs::CreateBlockOptions;
diff --git a/src/kudu/tablet/diskrowset-test.cc
b/src/kudu/tablet/diskrowset-test.cc
index 6081dc281..4a52e25c3 100644
--- a/src/kudu/tablet/diskrowset-test.cc
+++ b/src/kudu/tablet/diskrowset-test.cc
@@ -87,6 +87,7 @@ DECLARE_int32(tablet_history_max_age_sec);
DECLARE_bool(rowset_metadata_store_keys);
DECLARE_double(tablet_delta_store_major_compact_min_ratio);
DECLARE_int32(tablet_delta_store_minor_compact_max);
+DECLARE_uint64(all_delete_op_delta_file_cnt_for_compaction);
using std::is_sorted;
using std::make_tuple;
@@ -645,11 +646,22 @@ TEST_F(TestRowSet, TestCompactStores) {
}
TEST_F(TestRowSet, TestGCScheduleRedoFilesWithFullOfDeleteOP) {
+ // Enable the functionality introduced in the context of KUDU-3367 just
+ // for this test case.
+ FLAGS_all_delete_op_delta_file_cnt_for_compaction = 1;
+
// Make major delta compaction runnable even with tiny amount of data
accumulated
// across rowset's deltas.
FLAGS_tablet_delta_store_major_compact_min_ratio = 0.0001;
+
// Write the min/max keys to the rowset metadata. In this way, we can
simplify the
// test scenario by focusing on the REDO delta store files.
+ //
+ // TODO(aserbin): what to do if rowset metadata doesn't store the min/max
key?
+ // In such case, the major compaction like below simply fails,
+ // but that's just one reason for the functionality behind
+ // KUDU-3367 being defective; there is more: see KUDU-3619
+ // for details.
FLAGS_rowset_metadata_store_keys = true;
WriteTestRowSet();
