bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Merge 'sstable, test: log sstable name and pk when capping local_deletion_time ' from Kefu Chai

Browse Source 
in this series, we also print the sstable name and pk when writing a tombstone whose local_deletion_time (ldt for short) is greater than INT32_MAX which cannot be represented by an uint32_t.

Fixes #15015

Closes #15107

* github.com:scylladb/scylladb:
  sstable/writer: log sstable name and pk when capping ldt
  test: sstable_compaction_test: add a test for capped tombstone ldt
This commit is contained in:
Botond Dénes
2023-08-23 09:29:54 +03:00
parent f7505405f0 9c24be05c3
commit 139ba553b8
4 changed files with 44 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
sstables/mx/writer.cc
View File

@@ -669,6 +669,7 @@ private:
int32_t ldt = adjusted_local_deletion_time(t.deletion_time, capped);
if (capped) {
slogger.warn("Capping tombstone local_deletion_time {} to max {}", t.deletion_time.time_since_epoch().count(), ldt);
slogger.warn("Capping tombstone in sstable = {}, partition_key = {}", _sst.get_filename(), _partition_key->to_partition_key(_schema));
_sst.get_stats().on_capped_tombstone_deletion_time();
}
dt.local_deletion_time = ldt;

32
test/boost/sstable_compaction_test.cc
View File

@@ -976,9 +976,9 @@ SEASTAR_TEST_CASE(tombstone_purge_test) {
return m;
};
auto make_delete = [&] (partition_key key) {
auto make_delete = [&] (partition_key key, gc_clock::time_point deletion_time = gc_clock::now()) {
mutation m(s, key);
tombstone tomb(next_timestamp(), gc_clock::now());
tombstone tomb(next_timestamp(), deletion_time);
m.partition().apply(tomb);
return m;
};
@@ -1129,6 +1129,34 @@ SEASTAR_TEST_CASE(tombstone_purge_test) {
.produces(mut3)
.produces_end_of_stream();
}
{
// We use int32_t for representing a timestamp in seconds since the
// UNIX epoch. This timestamp "local_deletion_time" (ldt for short)
// notes the time at which a tombstone was created. It is used for
// purging the tombstone after gc_grace_seconds.
//
// If ldt is greater than INT32_MAX (2147483647), then it cannot be
// represented using a int32_t. probably more importantly, it
// represents a time point too far in the future -- after 19 Jan 2028.
// so the tombstone would practically live with us forever. so, the
// sstable writer just caps it to INT32_MAX - 1 when reading a
// tombstone with a TTL after this timepoint. and we also consider
// it as the indication of a problem and report it using the metrics
// named "scylla_sstables_capped_tombstone_deletion_time" which
// notes the total number of tombstones whose deletion_time breaches
// the limit.
//
// This test verifies that the metrics reflecting the number of
// tombstones with far-into-the-future ldts by inserting tombstones
// with a ldt greater than the date.
auto deletion_time = gc_clock::from_time_t(sstables::max_deletion_time + 1);
auto sst1 = make_sstable_containing(sst_gen,
{make_insert(alpha),
make_delete(alpha, deletion_time)},
validate::no);
auto result = compact({sst1}, {sst1});
BOOST_CHECK_EQUAL(1, sstables_stats::get_shard_stats().capped_tombstone_deletion_time);
}
});
}

19
test/lib/sstable_utils.cc
View File

@@ -52,11 +52,11 @@ sstables::shared_sstable make_sstable_containing(sstables::shared_sstable sst, l
return sst;
}
sstables::shared_sstable make_sstable_containing(std::function<sstables::shared_sstable()> sst_factory, std::vector<mutation> muts) {
return make_sstable_containing(sst_factory(), std::move(muts));
sstables::shared_sstable make_sstable_containing(std::function<sstables::shared_sstable()> sst_factory, std::vector<mutation> muts, validate do_validate) {
return make_sstable_containing(sst_factory(), std::move(muts), do_validate);
}
sstables::shared_sstable make_sstable_containing(sstables::shared_sstable sst, std::vector<mutation> muts) {
sstables::shared_sstable make_sstable_containing(sstables::shared_sstable sst, std::vector<mutation> muts, validate do_validate) {
tests::reader_concurrency_semaphore_wrapper semaphore;
schema_ptr s = muts[0].schema();
@@ -74,13 +74,14 @@ sstables::shared_sstable make_sstable_containing(sstables::shared_sstable sst, s
}
}
// validate the sstable
auto rd = assert_that(sst->as_mutation_source().make_reader_v2(s, semaphore.make_permit()));
for (auto&& m : merged) {
rd.produces(m);
if (do_validate) {
// validate the sstable
auto rd = assert_that(sst->as_mutation_source().make_reader_v2(s, semaphore.make_permit()));
for (auto&& m : merged) {
rd.produces(m);
}
rd.produces_end_of_stream();
}
rd.produces_end_of_stream();
return sst;
}

5
test/lib/sstable_utils.hh
View File

@@ -27,11 +27,12 @@
using namespace sstables;
using namespace std::chrono_literals;
using validate = bool_class<struct validate_tag>;
// Must be called in a seastar thread.
sstables::shared_sstable make_sstable_containing(std::function<sstables::shared_sstable()> sst_factory, lw_shared_ptr<replica::memtable> mt);
sstables::shared_sstable make_sstable_containing(sstables::shared_sstable sst, lw_shared_ptr<replica::memtable> mt);
sstables::shared_sstable make_sstable_containing(std::function<sstables::shared_sstable()> sst_factory, std::vector<mutation> muts);
sstables::shared_sstable make_sstable_containing(sstables::shared_sstable sst, std::vector<mutation> muts);
sstables::shared_sstable make_sstable_containing(std::function<sstables::shared_sstable()> sst_factory, std::vector<mutation> muts, validate do_validate = validate::yes);
sstables::shared_sstable make_sstable_containing(sstables::shared_sstable sst, std::vector<mutation> muts, validate do_validate = validate::yes);
inline future<> write_memtable_to_sstable_for_test(replica::memtable& mt, sstables::shared_sstable sst) {
return write_memtable_to_sstable(mt, sst, sst->manager().configure_writer("memtable"));