[#25319] docdb: Fixed Slice::Less

Summary:
`Slice::Less` is inconsistent with `Slice::compare` which is reproduced by the following test:
```
    str1 = "ABC";
    str2 = "";
    const auto s1 = Slice(str1);
    const auto s2 = Slice(str2);
    ASSERT_EQ(s1.Less(s2), s1.compare(s2) < 0);
```

Fixed `Slice::Less` and added SliceTest.OrderConsistency random test.
Jira: DB-14523

Test Plan: ybd --gtest_filter SliceTest.OrderConsistency -n 50 -- -p 1 for release/debug/asan/tsan builds

Reviewers: arybochkin, sergei

Reviewed By: arybochkin, sergei

Subscribers: sergei, ybase

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D40717

src/yb/rocksdb/db/compaction_iterator.cc

@@ -73,7 +73,7 @@ CompactionIterator::CompactionIterator(
 void CompactionIterator::AddLiveRanges(const std::vector<std::pair<Slice, Slice>>& ranges) {
   for (auto it = ranges.rbegin(); it != ranges.rend(); ++it) {
     const auto& range = *it;
-    DCHECK(range.first.Less(range.second));
+    DCHECK(range.second.empty() || range.first.Less(range.second)) << AsString(range);
     if (!live_key_ranges_stack_.empty()) {
       DCHECK(live_key_ranges_stack_.back().first.GreaterOrEqual(range.second));
     }

src/yb/util/slice-test.cc

@@ -37,7 +37,9 @@
 
 #include "yb/util/random_util.h"
 #include "yb/util/slice.h"
+#include "yb/util/test_util.h"
 #include "yb/util/tostring.h"
+#include "yb/util/tsan_util.h"
 
 using std::string;
 
@@ -90,13 +92,15 @@ size_t made_checks = 0;
 
 template <class... Args>
 void CheckLess(bool expected, const Slice& lhs, Args&&... rhs) {
+  static_assert(sizeof...(Args) > 0, "Args must not be empty");
   ASSERT_EQ(expected, lhs.Less(std::forward<Args>(rhs)...))
        << lhs.ToBuffer() << " vs " << AsString(std::tuple<Args...>(std::forward<Args>(rhs)...));
   ++made_checks;
 }
 
 template <size_t prefix>
 struct TestLessHelper<prefix, prefix, /* prefix_eq_len */ true> {
+  // Requires: lhs = rhs
   template <class... Args>
   static void Apply(const Slice& lhs, Args&&... rhs) {
     CheckLess(false, lhs, std::forward<Args>(rhs)...);
@@ -129,16 +133,18 @@ struct TestLessIteration<prefix, len, new_prefix, /* new_prefix_le_len= */ true>
 
 template <size_t prefix, size_t len>
 struct TestLessHelper<prefix, len, /* prefix_eq_len= */ false> {
+  // Requires: len = lhs.size(), lhs.starts_with(rhs), prefix = rhs.size(), prefix < len
   template <class... Args>
   static void Apply(const Slice& lhs, Args&&... rhs) {
-    CheckLess(true, lhs, std::forward<Args>(rhs)...);
+    CheckLess(false, lhs, std::forward<Args>(rhs)...);
 
     TestLessIteration<prefix, len, prefix + 1, (prefix < len)>::Apply(
         lhs, std::forward<Args>(rhs)...);
   }
 };
 
 template <size_t prefix, size_t len, class... Args>
+// Requires: len = lhs.size(), lhs.starts_with(rhs), prefix = rhs.size()
 void TestLess(const Slice& lhs, Args&&... rhs) {
   char kMinChar = '\x00';
   char kMaxChar = '\xff';
@@ -165,4 +171,57 @@ TEST(SliceTest, Less) {
   ASSERT_EQ(made_checks, (1ULL << kLen) * 3 - 1);
 }
 
+template <class... Args>
+void TestLessComponents(
+    const bool expected, const size_t component_size, const Slice& lhs, const Slice& rhs0,
+    Args&&... rhs) {
+  CheckLess(expected, lhs, rhs0, std::forward<Args>(rhs)...);
+  NO_PENDING_FATALS();
+  if (RandomActWithProbability(0.9)) {
+    CheckLess(expected, lhs, rhs0, Slice(""), std::forward<Args>(rhs)...);
+    NO_PENDING_FATALS();
+  }
+  if (rhs0.size() > component_size) {
+    CheckLess(
+        expected, lhs, rhs0.Prefix(rhs0.size() - component_size), rhs0.Suffix(component_size),
+        std::forward<Args>(rhs)...);
+  }
+}
+
+void TestOrder(const std::string& str1, const std::string& str2, const size_t num_components) {
+  const auto s1 = Slice(str1);
+  const auto s2 = Slice(str2);
+  auto component_size = std::max<size_t>(s2.size() / num_components, 1);
+  TestLessComponents(s1.compare(s2) < 0, component_size, s1, s2);
+  component_size = std::max<size_t>(s1.size() / num_components, 1);
+  TestLessComponents(s2.compare(s1) < 0, component_size, s2, s1);
+}
+
+// Test that Slice::Less order is consistent with Slice::compare.
+TEST(SliceTest, OrderConsistency) {
+  constexpr auto kNumIters = ReleaseVsDebugVsAsanVsTsan(100000, 100000, 50000, 50000);
+  constexpr auto kMaxLength = 256;
+  constexpr auto kMaxComponents = 10;
+
+  TestOrder("ABC", "", /* num_components = */ kMaxComponents);
+  TestOrder("ABCD", "ABC", /* num_components = */ kMaxComponents);
+  TestOrder("ABC", "ABC", /* num_components = */ kMaxComponents);
+
+  std::string str1;
+  std::string str2;
+
+  for (int iter = 0; iter < kNumIters; ++iter) {
+    const auto len1 = RandomUniformInt<size_t>(0, kMaxLength);
+    const auto len2 = RandomUniformInt<size_t>(0, kMaxLength);
+    str1 = RandomHumanReadableString(len1);
+    str2 = RandomHumanReadableString(len2);
+
+    const auto num_components = 1 + kMaxComponents * iter / kNumIters;
+
+    TestOrder(str1, str2, num_components);
+    NO_PENDING_FATALS();
+  }
+  LOG(INFO) << "made_checks: " << made_checks;
+}
+
 } // namespace yb

src/yb/util/slice.h

@@ -116,6 +116,7 @@ class Slice {
   // Return true iff the length of the referenced data is zero
   bool empty() const { return begin_ == end_; }
 
+  // GreaterOrEqual and Less compare this slice with concatenation of slices arg0 + args.
   template <class... Args>
   bool GreaterOrEqual(const Slice& arg0, Args&&... args) const {
     return !Less(arg0, std::forward<Args>(args)...);
@@ -299,10 +300,6 @@ class Slice {
  private:
   friend bool operator==(const Slice& x, const Slice& y);
 
-  bool DoLess() const {
-    return !empty();
-  }
-
   template <class... Args>
   bool DoLess(const Slice& arg0, Args&&... args) const {
     auto arg0_size = arg0.size();
@@ -315,7 +312,12 @@ class Slice {
       return cmp < 0;
     }
 
-    return Slice(begin_ + arg0_size, end_).DoLess(std::forward<Args>(args)...);
+    if constexpr (sizeof...(Args)) {
+      return Slice(begin_ + arg0_size, end_).DoLess(std::forward<Args>(args)...);
+    }
+
+    // args is absent and this.starts_with(arg0) => definitely not less than arg0.
+    return false;
   }
 
   static bool MemEqual(const void* a, const void* b, size_t n) {