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.

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

storage: handle IngestSST using segment split #6378

Merged
merged 26 commits into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
97e1f08
Pick impl from older branch
breezewish Nov 28, 2022
678f40f
Check external files are not overlapped in ingestFiles
breezewish Nov 28, 2022
39d2917
Fix tests
breezewish Nov 28, 2022
1b97371
storage: Add more checks for ingest
breezewish Nov 28, 2022
09aee07
Merge remote-tracking branch 'origin/master' into wenxuan/ingest_chec…
breezewish Nov 28, 2022
a91ded3
Reformat
breezewish Nov 28, 2022
9a3e9d4
Revert the segment abandon change
breezewish Nov 28, 2022
52b7427
Merge remote-tracking branch 'origin/master' into wenxuan/ingest-by-s…
breezewish Nov 28, 2022
85b6ac2
Merge commit 'a91ded340ded7a19629e355e43b0d1784b62bbf7' into wenxuan/…
breezewish Nov 28, 2022
8514195
Address comments
breezewish Nov 29, 2022
cab390b
Cherry pick upstream changes
breezewish Nov 29, 2022
1eac9ae
Fix clang-tidy lints
breezewish Nov 29, 2022
640b397
Merge branch 'master' into wenxuan/ingest_check_not_overlap
ti-chi-bot Nov 29, 2022
525db1e
storage: Accepts a snapshot for segment replaceData API
breezewish Nov 29, 2022
52cf4be
Merge branch 'master' into wenxuan/replace_data_snapshot
breezewish Nov 29, 2022
5648b3c
Merge commit '640b3974c6aa6880d4a65dcfd9b75714b9408ca8' into wenxuan/…
breezewish Nov 29, 2022
edd09e4
Fix typos
breezewish Nov 29, 2022
37c7eba
Merge commit '52cf4be99306a6b4aef4e46a2fdae057bdfb499f' into wenxuan/…
breezewish Nov 29, 2022
2e7122d
Merge commit 'f57ec4be15f1eb3a5e459fe5850b2a1b6ce678d9' into wenxuan/…
breezewish Nov 30, 2022
64b1e94
Fix merge issue
breezewish Nov 30, 2022
49064f4
Add concurrent write & ingest test
breezewish Nov 30, 2022
1a1f7eb
Add debug logs
breezewish Nov 30, 2022
f093c49
Address comments
breezewish Dec 1, 2022
d631a73
Unify the name of splitByIngest and splitByReplace (they are the same)
breezewish Dec 1, 2022
e4b5c02
storage: Simplify segmentIngestData
breezewish Dec 1, 2022
742f389
Merge branch 'master' into wenxuan/ingest-by-split-2
flowbehappy Dec 1, 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
2 changes: 1 addition & 1 deletion dbms/src/Common/CurrentMetrics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@
M(DT_SnapshotOfReadRaw) \
M(DT_SnapshotOfSegmentSplit) \
M(DT_SnapshotOfSegmentMerge) \
M(DT_SnapshotOfSegmentIngest) \
M(DT_SnapshotOfDeltaMerge) \
M(DT_SnapshotOfDeltaCompact) \
M(DT_SnapshotOfPlaceIndex) \
M(DT_SnapshotOfSegmentIngest) \
M(IOLimiterPendingBgWriteReq) \
M(IOLimiterPendingFgWriteReq) \
M(IOLimiterPendingBgReadReq) \
Expand Down
2 changes: 2 additions & 0 deletions dbms/src/Common/FailPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ std::unordered_map<String, std::shared_ptr<FailPointChannel>> FailPointHelper::f
M(force_context_path) \
M(force_slow_page_storage_snapshot_release) \
M(force_change_all_blobs_to_read_only) \
M(force_ingest_via_delta) \
M(force_ingest_via_replace) \
M(unblock_query_init_after_write) \
M(exception_in_merged_task_init) \
M(force_fail_in_flush_region_data)
Expand Down
2 changes: 2 additions & 0 deletions dbms/src/Common/ProfileEvents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
M(DMCleanReadRows) \
M(DMSegmentIsEmptyFastPath) \
M(DMSegmentIsEmptySlowPath) \
M(DMSegmentIngestDataByReplace) \
M(DMSegmentIngestDataIntoDelta) \
\
M(FileFSync) \
\
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Common/TiFlashMetrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ namespace DB
F(type_seg_split_bg, {"type", "seg_split_bg"}), \
F(type_seg_split_fg, {"type", "seg_split_fg"}), \
F(type_seg_split_ingest, {"type", "seg_split_ingest"}), \
F(type_seg_merge_bg_gc, {"type", "type_seg_merge_bg_gc"}), \
F(type_seg_merge_bg_gc, {"type", "seg_merge_bg_gc"}), \
F(type_place_index_update, {"type", "place_index_update"})) \
M(tiflash_storage_subtask_duration_seconds, "Bucketed histogram of storage's sub task duration", Histogram, \
F(type_delta_merge_bg, {{"type", "delta_merge_bg"}}, ExpBuckets{0.001, 2, 20}), \
Expand Down
5 changes: 3 additions & 2 deletions dbms/src/Storages/DeltaMerge/Delta/DeltaValueSpace.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class DeltaValueSpace
, private boost::noncopyable
{
public:
using Lock = std::unique_lock<std::mutex>;
using Lock = std::unique_lock<std::recursive_mutex>;

private:
/// column files in `persisted_file_set` are all persisted in disks and can be restored after restart.
Expand Down Expand Up @@ -94,7 +94,8 @@ class DeltaValueSpace
DeltaIndexPtr delta_index;

// Protects the operations in this instance.
mutable std::mutex mutex;
// It is a recursive_mutex because the lock may be also used by the parent segment as its update lock.
mutable std::recursive_mutex mutex;
Copy link
Member Author

@breezewish breezewish Nov 30, 2022

Choose a reason for hiding this comment

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

Note: This recursive mutex happens when:

  1. DeltaMergeStore::segmentIngestData grabs a segment update lock
  2. DeltaMergeStore::segmentIngestData discovered the segment is not empty and then ingest files into the delta.
    • DeltaValueSpace::ingestColumnFiles grabs the same lock to write to delta VS.

The real problem is we do not distinguish the segment update lock with the delta lock. When both lock is needed, they are conflicted. Also, I did not changed the ingestColumnFiles API to allow passing an external lock, because other APIs in the DeltaVS do not have this pattern and I think it may not be a good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually, in DeltaMergeStore::segmentApplyXXX we will acquire the segment update lock and won't change the DeltaVS inside the "apply".
segmentIngestData brings a special pattern that may change the DeltaVS inside its "apply". But I didn't figure out a better solution


LoggerPtr log;

Expand Down
5 changes: 3 additions & 2 deletions dbms/src/Storages/DeltaMerge/Delta/MemTableSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ void MemTableSet::appendDeleteRange(const RowKeyRange & delete_range)

void MemTableSet::ingestColumnFiles(const RowKeyRange & range, const ColumnFiles & new_column_files, bool clear_data_in_range)
{
for (const auto & f : new_column_files)
RUNTIME_CHECK(f->isBigFile());

// Prepend a DeleteRange to clean data before applying column files
if (clear_data_in_range)
{
Expand All @@ -242,9 +245,7 @@ void MemTableSet::ingestColumnFiles(const RowKeyRange & range, const ColumnFiles
}

for (const auto & f : new_column_files)
{
appendColumnFileInner(f);
}
}

ColumnFileSetSnapshotPtr MemTableSet::createSnapshot(const StorageSnapshotPtr & storage_snap, bool disable_sharing)
Expand Down
35 changes: 19 additions & 16 deletions dbms/src/Storages/DeltaMerge/DeltaMergeStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ class DeltaMergeStore : private boost::noncopyable
{
ForegroundWrite,
Background,
IngestBySplit,
ForIngest,
};

/**
Expand Down Expand Up @@ -510,29 +510,31 @@ class DeltaMergeStore : private boost::noncopyable
SegmentSnapshotPtr segment_snap = nullptr);

/**
* Discard all data in the segment, and use the specified DMFile as the stable instead.
* The specified DMFile is safe to be shared for multiple segments.
* Ingest a DMFile into the segment, optionally causing a new segment being created.
*
* Note 1: This function will not enable GC for the new_stable_file for you, in case of you may want to share the same
* stable file for multiple segments. It is your own duty to enable GC later.
*
* Note 2: You must ensure the specified new_stable_file has been managed by the storage pool, and has been written
* to the PageStorage's data. Otherwise there will be exceptions.
*
* Note 3: This API is subjected to be changed in future, as it relies on the knowledge that all current data
* in this segment is useless, which is a pretty tough requirement.
* Note 1: You must ensure the DMFile is not shared in multiple segments.
* Note 2: You must enable the GC for the DMFile by yourself.
* Note 3: You must ensure the DMFile has been managed by the storage pool, and has been written
* to the PageStorage's data.

* @param clear_all_data_in_segment Whether all data in the segment should be discarded.
* @returns one of:
* - A new segment: A new segment is created for containing the data
* - The same segment as passed in: Data is ingested into the delta layer of current segment
* - nullptr: when there are errors
*/
SegmentPtr segmentDangerouslyReplaceData(
SegmentPtr segmentIngestData(
DMContext & dm_context,
const SegmentPtr & segment,
const DMFilePtr & data_file);
const DMFilePtr & data_file,
bool clear_all_data_in_segment);

// isSegmentValid should be protected by lock on `read_write_mutex`
inline bool isSegmentValid(const std::shared_lock<std::shared_mutex> &, const SegmentPtr & segment)
bool isSegmentValid(const std::shared_lock<std::shared_mutex> &, const SegmentPtr & segment)
{
return doIsSegmentValid(segment);
}
inline bool isSegmentValid(const std::unique_lock<std::shared_mutex> &, const SegmentPtr & segment)
bool isSegmentValid(const std::unique_lock<std::shared_mutex> &, const SegmentPtr & segment)
{
return doIsSegmentValid(segment);
}
Expand All @@ -559,7 +561,8 @@ class DeltaMergeStore : private boost::noncopyable
DMContext & dm_context,
const SegmentPtr & segment,
const RowKeyRange & ingest_range,
const DMFilePtr & file);
const DMFilePtr & file,
bool clear_data_in_range);

bool updateGCSafePoint();

Expand Down
Loading