Skip to content

Commit

Permalink
KUDU-3619 disable KUDU-3367 behavior by default
Browse files Browse the repository at this point in the history
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 <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
  • Loading branch information
alexeyserbin committed Sep 25, 2024
1 parent 60a5cb6 commit 3666d20
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/kudu/tablet/delta_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "kudu/tablet/delta_tracker.h"

#include <algorithm>
#include <limits>
#include <optional>
#include <ostream>
#include <set>
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions src/kudu/tablet/diskrowset-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 3666d20

Please sign in to comment.