This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/branch-1.17.x by this push:
     new 05043e6ab KUDU-3619 disable KUDU-3367 behavior by default
05043e6ab is described below

commit 05043e6aba6ab45c1b77de9f0762de3dfc5a54c0
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>
    (cherry picked from commit 3666d2026d48adb5ff636321ef22320a8af5facb)
      Conflicts:
        src/kudu/tablet/delta_tracker.cc
    Reviewed-on: http://gerrit.cloudera.org:8080/21855
    Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
---
 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 d336867fd..962c83d8e 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 <mutex>
 #include <optional>
 #include <ostream>
@@ -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.");
 
 namespace kudu {
 
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();

Reply via email to