Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean up segments with large space amplification ratio generated by logical split #5860

Merged
merged 8 commits into from
Sep 14, 2022
Merged

clean up segments with large space amplification ratio generated by logical split #5860

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
82319ac
merge delta if segment valid data is too small
lidezhu Sep 13, 2022
f6ad6a2
add more log info for gc thread
lidezhu Sep 13, 2022
9b10b21
add unit test
lidezhu Sep 13, 2022
58f5b41
fix conflict
lidezhu Sep 13, 2022
9490589
small fix for typo
lidezhu Sep 13, 2022
75f51fa
address comment
lidezhu Sep 13, 2022
d3f00d8
check whether this segment share dtfiles with neighbor segments befor…
lidezhu Sep 14, 2022
308b9c4
Merge branch 'master' into merge-delta-for-logical-split
lidezhu Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 75 additions & 15 deletions dbms/src/Storages/DeltaMerge/DeltaMergeStore_InternalBg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,32 @@ bool DeltaMergeStore::handleBackgroundTask(bool heavy)

namespace GC
{
enum Type
{
Unknown,
TooManyDeleteRange,
TooMuchOutOfRange,
TooManyInvalidVersion,
};

static std::string toString(Type type)
{
switch (type)
{
case TooManyDeleteRange:
return "TooManyDeleteRange";
case TooMuchOutOfRange:
return "TooMuchOutOfRange";
case TooManyInvalidVersion:
return "TooManyInvalidVersion";
default:
return "Unknown";
}
}

// Returns true if it needs gc.
// This is for optimization purpose, does not mean to be accurate.
bool shouldCompactStable(const SegmentPtr & seg, DB::Timestamp gc_safepoint, double ratio_threshold, const LoggerPtr & log)
bool shouldCompactStableWithTooManyInvalidVersion(const SegmentPtr & seg, DB::Timestamp gc_safepoint, double ratio_threshold, const LoggerPtr & log)
{
// Always GC.
if (ratio_threshold < 1.0)
Expand All @@ -271,7 +294,7 @@ bool shouldCompactStable(const SegmentPtr & seg, DB::Timestamp gc_safepoint, dou
return false;
}

bool shouldCompactDeltaWithStable(const DMContext & context, const SegmentSnapshotPtr & snap, const RowKeyRange & segment_range, double ratio_threshold, const LoggerPtr & log)
bool shouldCompactDeltaWithStable(const DMContext & context, const SegmentSnapshotPtr & snap, const RowKeyRange & segment_range, double invalid_data_ratio_threshold, const LoggerPtr & log)
{
auto actual_delete_range = snap->delta->getSquashDeleteRange().shrink(segment_range);
if (actual_delete_range.none())
Expand All @@ -291,8 +314,26 @@ bool shouldCompactDeltaWithStable(const DMContext & context, const SegmentSnapsh
// because before write apply snapshot file, it will write a delete range first, and will meet the following gc criteria.
// But the cost should be really minor because merge delta on an empty segment should be very fast.
// What's more, we can ignore this kind of delete range in future to avoid this extra gc.
bool should_compact = (delete_rows >= stable_rows * ratio_threshold) || (delete_bytes >= stable_bytes * ratio_threshold);
return should_compact;
return (delete_rows >= stable_rows * invalid_data_ratio_threshold) || (delete_bytes >= stable_bytes * invalid_data_ratio_threshold);
}

bool shouldCompactStableWithTooMuchDataOutOfSegmentRange(const SegmentSnapshotPtr & snap, double invalid_data_ratio_threshold, const LoggerPtr & log)
{
auto valid_rows = snap->stable->getRows();
auto valid_bytes = snap->stable->getBytes();

const auto & dm_files = snap->stable->getDMFiles();
size_t total_rows = 0;
size_t total_bytes = 0;
for (const auto & file : dm_files)
{
total_rows += file->getRows();
total_bytes += file->getBytes();
}

LOG_FMT_TRACE(log, "valid_rows [{}], valid_bytes [{}] total_rows [{}] total_bytes [{}]", valid_rows, valid_bytes, total_rows, total_bytes);

return (valid_rows < total_rows * (1 - invalid_data_ratio_threshold)) || (valid_bytes < total_bytes * (1 - invalid_data_ratio_threshold));
Copy link
Member

@breezewish breezewish Sep 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean, if we have logical split enabled, then logical split will be nearly useless (and becomes very similar to physical split), because all segments split-out by logical split usually only contains 50% data (which < the 70% threshold)?

Consider the workload:

  1. Write a lot of data to one segment
  2. Segment is logical split into two
  3. (No further writes to these two segment)

In this PR, the two segment each contains 50% data, so that they both trigger GC.

However, actually in this workload GC will not reclaim further spaces, because all data in the underlying DTFile is "useful".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the heavy work is moved to background gc thread. And I thought this is exactly the solution that we talked about previously(

Copy link
Member

@breezewish breezewish Sep 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, but may be a better way would be counting by DTFiles, for example, check whether a DTFile is < 70% utilized. When a DTFile is < 70% utilized, it means delta-merging all segments who use this DTFile can result in reclaiming 30% space. Otherwise delta-merging these segments will not have notable benefits.

However, the newly proposed check could be more complex (it definitely needs iterating all segments for multiple rounds) and I'm not sure whether covering this case is useful. @JaySon-Huang @flowbehappy What do you think?

Copy link
Contributor Author

@lidezhu lidezhu Sep 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. This is a better and more complex solution.

Copy link
Contributor Author

@lidezhu lidezhu Sep 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a check for whether this segment share any DTFile with neighbor segments before check the invalid data ratio.
So for segments generated by logical split, it will only do clean work when there is no neighbor sharing the same DTFile.

For DTFile which is shared by two segments, this is exactly the correct behavior we want.

For DTFile which is shared by more than two segments,

  1. this is really a rare case, because it is hard to do yet another logical split on a segment which is generated by logical split.(Check Segment::getSplitPointFast);
  2. even it happens, if one of the multiple segments is updated and on longer ref the shared DTFile, it is beneficial to do the clean work on the remaining segments;

So I think the current solution is good enough for our purpose.

}
} // namespace GC

Expand Down Expand Up @@ -370,17 +411,30 @@ UInt64 DeltaMergeStore::onSyncGc(Int64 limit)
try
{
// Check whether we should apply gc on this segment
auto invalid_data_ratio_threshold = global_context.getSettingsRef().dt_bg_gc_delta_delete_ratio_to_trigger_gc;
RUNTIME_ASSERT(invalid_data_ratio_threshold >= 0 && invalid_data_ratio_threshold <= 1);
bool should_compact = false;
GC::Type gc_type = GC::Type::Unknown;
if (GC::shouldCompactDeltaWithStable(
*dm_context,
segment_snap,
segment_range,
global_context.getSettingsRef().dt_bg_gc_delta_delete_ratio_to_trigger_gc,
invalid_data_ratio_threshold,
log))
{
should_compact = true;
gc_type = GC::Type::TooManyDeleteRange;
}
else if (!segment->isValidDataRatioChecked())
{
segment->setValidDataRatioChecked();
if (GC::shouldCompactStableWithTooMuchDataOutOfSegmentRange(segment_snap, invalid_data_ratio_threshold, log))
{
should_compact = true;
gc_type = GC::Type::TooMuchOutOfRange;
}
}
else if (segment->getLastCheckGCSafePoint() < gc_safe_point)
else if (!should_compact && (segment->getLastCheckGCSafePoint() < gc_safe_point))
{
// Avoid recheck this segment when gc_safe_point doesn't change regardless whether we trigger this segment's DeltaMerge or not.
// Because after we calculate StableProperty and compare it with this gc_safe_point,
Expand All @@ -394,11 +448,15 @@ UInt64 DeltaMergeStore::onSyncGc(Int64 limit)
if (!segment->getStable()->isStablePropertyCached())
segment->getStable()->calculateStableProperty(*dm_context, segment_range, isCommonHandle());

should_compact = GC::shouldCompactStable(
segment,
gc_safe_point,
global_context.getSettingsRef().dt_bg_gc_ratio_threhold_to_trigger_gc,
log);
if (GC::shouldCompactStableWithTooManyInvalidVersion(
segment,
gc_safe_point,
global_context.getSettingsRef().dt_bg_gc_ratio_threhold_to_trigger_gc,
log))
{
should_compact = true;
gc_type = GC::Type::TooManyInvalidVersion;
}
}
bool finish_gc_on_segment = false;
if (should_compact)
Expand All @@ -412,17 +470,19 @@ UInt64 DeltaMergeStore::onSyncGc(Int64 limit)
finish_gc_on_segment = true;
LOG_FMT_DEBUG(
log,
"Finish GC-merge-delta, segment={} table={}",
"Finish GC-merge-delta, segment={} table={}, gc_type={}",
segment->simpleInfo(),
table_name);
table_name,
GC::toString(gc_type));
}
else
{
LOG_FMT_DEBUG(
log,
"GC aborted, segment={} table={}",
"GC aborted, segment={} table={}, gc_type={}",
segment->simpleInfo(),
table_name);
table_name,
GC::toString(gc_type));
}
}
if (!finish_gc_on_segment)
Expand Down
8 changes: 8 additions & 0 deletions dbms/src/Storages/DeltaMerge/Segment.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,9 @@ class Segment : private boost::noncopyable
bool isSplitForbidden() const { return split_forbidden; }
void forbidSplit() { split_forbidden = true; }

bool isValidDataRatioChecked() const { return check_valid_data_ratio.load(std::memory_order_relaxed); }
void setValidDataRatioChecked() { check_valid_data_ratio.store(true, std::memory_order_relaxed); }

void drop(const FileProviderPtr & file_provider, WriteBatches & wbs);

bool isFlushing() const { return delta->isFlushing(); }
Expand Down Expand Up @@ -424,6 +427,11 @@ class Segment : private boost::noncopyable
const StableValueSpacePtr stable;

bool split_forbidden = false;
// After logical split, it is very possible that only half of the data in the segment's DTFile is valid for this segment.
// So we want to do merge delta on this kind of segment to clean out the invalid data.
// This involves to check the valid data ratio in the background gc thread,
// and to avoid doing this check repeatedly, we add this flag to indicate whether the valid data ratio has already been checked.
std::atomic<bool> check_valid_data_ratio = false;
lidezhu marked this conversation as resolved.
Show resolved Hide resolved

LoggerPtr log;
};
Expand Down
45 changes: 45 additions & 0 deletions dbms/src/Storages/DeltaMerge/tests/gtest_segment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@

#include <future>

namespace CurrentMetrics
{
extern const Metric DT_SnapshotOfDeltaMerge;
} // namespace CurrentMetrics

namespace DB
{

Expand All @@ -35,6 +40,10 @@ extern const char force_segment_logical_split[];

namespace DM
{
namespace GC
{
bool shouldCompactStableWithTooMuchDataOutOfSegmentRange(const SegmentSnapshotPtr & snap, double invalid_data_ratio_threshold, const LoggerPtr & log);
}
namespace tests
{
class SegmentOperationTest : public SegmentTestBasic
Expand Down Expand Up @@ -432,6 +441,42 @@ try
}
CATCH

TEST_F(SegmentOperationTest, GCCheckAfterSegmentLogicalSplit)
try
{
{
SegmentTestOptions options;
options.db_settings.dt_segment_stable_pack_rows = 100;
reloadWithOptions(options);
}

writeSegment(DELTA_MERGE_FIRST_SEGMENT_ID, 1000);
flushSegmentCache(DELTA_MERGE_FIRST_SEGMENT_ID);
mergeSegmentDelta(DELTA_MERGE_FIRST_SEGMENT_ID);
auto invalid_data_ratio_threshold = dm_context->db_context.getSettingsRef().dt_bg_gc_delta_delete_ratio_to_trigger_gc;
{
auto segment = segments[DELTA_MERGE_FIRST_SEGMENT_ID];
auto snap = segment->createSnapshot(*dm_context, /* for_update */ true, CurrentMetrics::DT_SnapshotOfDeltaMerge);
ASSERT_FALSE(GC::shouldCompactStableWithTooMuchDataOutOfSegmentRange(snap, invalid_data_ratio_threshold, log));
}

FailPointHelper::enableFailPoint(FailPoints::force_segment_logical_split);
auto new_seg_id_opt = splitSegment(DELTA_MERGE_FIRST_SEGMENT_ID);
ASSERT_TRUE(new_seg_id_opt.has_value());

{
auto segment = segments[DELTA_MERGE_FIRST_SEGMENT_ID];
auto snap = segment->createSnapshot(*dm_context, /* for_update */ true, CurrentMetrics::DT_SnapshotOfDeltaMerge);
ASSERT_TRUE(GC::shouldCompactStableWithTooMuchDataOutOfSegmentRange(snap, invalid_data_ratio_threshold, log));
}
{
auto segment = segments[new_seg_id_opt.value()];
auto snap = segment->createSnapshot(*dm_context, /* for_update */ true, CurrentMetrics::DT_SnapshotOfDeltaMerge);
ASSERT_TRUE(GC::shouldCompactStableWithTooMuchDataOutOfSegmentRange(snap, invalid_data_ratio_threshold, log));
}
}
CATCH

TEST_F(SegmentOperationTest, Issue5570)
try
{
Expand Down