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

Fix max_caps may not been update after GC #4323

Merged
merged 9 commits into from
Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
49 changes: 39 additions & 10 deletions dbms/src/Storages/Page/V3/BlobStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,18 +192,36 @@ PageEntriesEdit BlobStore::write(DB::WriteBatch & wb, const WriteLimiterPtr & wr

void BlobStore::remove(const PageEntriesV3 & del_entries)
{
std::set<BlobFileId> blob_updated;
for (const auto & entry : del_entries)
{
blob_updated.insert(entry.file_id);
// External page size is 0
if (entry.size == 0)
{
continue;
// throw Exception(fmt::format("Invaild entry. entry size 0. [id={}] [offset={}]",
// entry.file_id,
// entry.offset));
}
removePosFromStats(entry.file_id, entry.offset, entry.size);
}

// After we remove postion of blob, we need recalculate the blob.
for (const auto & blob_id : blob_updated)
{
const auto & stat = blob_stats.blobIdToStat(blob_id,
/*restore_if_not_exist*/ false,
/*ignore_not_exist*/ true);

// Some of blob may been removed.
// So if we can't use id find blob, just ignore it.
if (stat)
{
{
auto lock = stat->lock();
stat->recalculateCapability();
}
LOG_FMT_TRACE(log, "Blob begin to recalculate capability [blob_id={}]", blob_id);
jiaqizho marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

std::pair<BlobFileId, BlobFileOffset> BlobStore::getPosFromStats(size_t size)
Expand Down Expand Up @@ -747,6 +765,7 @@ void BlobStore::BlobStats::restore()
for (const auto & stat : stats_map)
{
stat->recalculateSpaceMap();
stat->recalculateCapability();
jiaqizho marked this conversation as resolved.
Show resolved Hide resolved
max_restored_file_id = std::max(stat->id, max_restored_file_id);
existing_file_ids.insert(stat->id);
}
Expand Down Expand Up @@ -832,7 +851,7 @@ void BlobStore::BlobStats::eraseStat(BlobFileId blob_file_id, const std::lock_gu
eraseStat(std::move(stat), lock);
}

std::pair<BlobStatPtr, BlobFileId> BlobStore::BlobStats::chooseStat(size_t buf_size, UInt64 file_limit_size, const std::lock_guard<std::mutex> &)
std::pair<BlobStatPtr, BlobFileId> BlobStore::BlobStats::chooseStat(size_t buf_size, UInt64 /*file_limit_size*/, const std::lock_guard<std::mutex> &)
{
BlobStatPtr stat_ptr = nullptr;
double smallest_valid_rate = 2;
Expand All @@ -847,10 +866,10 @@ std::pair<BlobStatPtr, BlobFileId> BlobStore::BlobStats::chooseStat(size_t buf_s
{
if (!stat->isReadOnly()
&& stat->sm_max_caps >= buf_size
&& stat->sm_total_size + buf_size < file_limit_size
// && stat->sm_total_size + buf_size < file_limit_size
JaySon-Huang marked this conversation as resolved.
Show resolved Hide resolved
&& stat->sm_valid_rate < smallest_valid_rate)
{
smallest_valid_rate = stat->sm_valid_size;
smallest_valid_rate = stat->sm_valid_rate;
stat_ptr = stat;
}
}
Expand All @@ -863,7 +882,7 @@ std::pair<BlobStatPtr, BlobFileId> BlobStore::BlobStats::chooseStat(size_t buf_s
return std::make_pair(stat_ptr, INVALID_BLOBFILE_ID);
}

BlobStatPtr BlobStore::BlobStats::blobIdToStat(BlobFileId file_id, bool restore_if_not_exist)
BlobStatPtr BlobStore::BlobStats::blobIdToStat(BlobFileId file_id, bool restore_if_not_exist, bool ignore_not_exist)
{
auto guard = lock();
for (auto & stat : stats_map)
Expand All @@ -880,9 +899,14 @@ BlobStatPtr BlobStore::BlobStats::blobIdToStat(BlobFileId file_id, bool restore_
return createStatNotCheckingRoll(file_id, guard);
}

throw Exception(fmt::format("Can't find BlobStat with [blob_id={}]",
file_id),
ErrorCodes::LOGICAL_ERROR);
if (!ignore_not_exist)
{
throw Exception(fmt::format("Can't find BlobStat with [blob_id={}]",
file_id),
ErrorCodes::LOGICAL_ERROR);
}

return nullptr;
}

/*********************
Expand Down Expand Up @@ -962,5 +986,10 @@ void BlobStore::BlobStats::BlobStat::recalculateSpaceMap()
sm_valid_rate = valid_size * 1.0 / total_size;
}

void BlobStore::BlobStats::BlobStat::recalculateCapability()
{
sm_max_caps = smap->updateAccurateMaxCapacity();
}

} // namespace PS::V3
} // namespace DB
10 changes: 9 additions & 1 deletion dbms/src/Storages/Page/V3/BlobStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ class BlobStore : private Allocator<false>
* We still need to recalculate a `sm_total_size`/`sm_valid_size`/`sm_valid_rate`.
*/
void recalculateSpaceMap();

/**
* After GC we removed out of date data.
* But we can't calculate the `sm_max_cap`.It can only be updated through spacemap.
*
* Also after restore, we need call this method to recalculate the `sm_max_cap`.
*/
jiaqizho marked this conversation as resolved.
Show resolved Hide resolved
void recalculateCapability();
jiaqizho marked this conversation as resolved.
Show resolved Hide resolved
};

using BlobStatPtr = std::shared_ptr<BlobStat>;
Expand Down Expand Up @@ -143,7 +151,7 @@ class BlobStore : private Allocator<false>
*/
std::pair<BlobStatPtr, BlobFileId> chooseStat(size_t buf_size, UInt64 file_limit_size, const std::lock_guard<std::mutex> &);

BlobStatPtr blobIdToStat(BlobFileId file_id, bool restore_if_not_exist = false);
BlobStatPtr blobIdToStat(BlobFileId file_id, bool restore_if_not_exist = false, bool ignore_not_exist = false);

std::list<BlobStatPtr> getStats() const
{
Expand Down
4 changes: 4 additions & 0 deletions dbms/src/Storages/Page/V3/spacemap/SpaceMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ class SpaceMap
*/
virtual UInt64 getRightMargin() = 0;

/**
* Get the max of capability of space map.
jiaqizho marked this conversation as resolved.
Show resolved Hide resolved
*/
virtual UInt64 updateAccurateMaxCapacity() = 0;

/**
* Return the size of file and the size contains valid data.
Expand Down
28 changes: 28 additions & 0 deletions dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,34 @@ std::pair<UInt64, UInt64> RBTreeSpaceMap::searchInsertOffset(size_t size)
return std::make_pair(offset, max_cap);
}

UInt64 RBTreeSpaceMap::updateAccurateMaxCapacity()
{
struct rb_node * node = nullptr;
struct SmapRbEntry * entry;
UInt64 max_offset = 0;
UInt64 max_cap = 0;

node = rb_tree_first(&rb_tree->root);
if (node == nullptr)
{
return max_cap;
}

for (; node != nullptr; node = rb_tree_next(node))
{
entry = node_to_entry(node);
if (entry->count > max_cap)
{
max_offset = entry->start;
max_cap = entry->count;
}
}

biggest_range = max_offset;
biggest_cap = max_cap;
return max_cap;
}

std::pair<UInt64, UInt64> RBTreeSpaceMap::getSizes() const
{
struct rb_node * node = rb_tree_last(&rb_tree->root);
Expand Down
2 changes: 2 additions & 0 deletions dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class RBTreeSpaceMap

std::pair<UInt64, UInt64> searchInsertOffset(size_t size) override;

UInt64 updateAccurateMaxCapacity() override;

std::pair<UInt64, UInt64> getSizes() const override;

UInt64 getRightMargin() override;
Expand Down
19 changes: 19 additions & 0 deletions dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,25 @@ class STDMapSpaceMap
return std::make_pair(offset, hint_biggest_cap);
}

UInt64 updateAccurateMaxCapacity() override
{
UInt64 max_offset = 0;
UInt64 max_cap = 0;

for (const auto & [start, size] : free_map)
{
if (size > max_cap)
{
max_cap = size;
max_offset = start;
}
}
hint_biggest_offset = max_offset;
hint_biggest_cap = max_cap;

return max_cap;
}

bool markFreeImpl(UInt64 offset, size_t length) override
{
auto it = free_map.find(offset);
Expand Down
19 changes: 19 additions & 0 deletions dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,25 @@ TEST_P(SpaceMapTest, TestGetSizes)
}
}


TEST_P(SpaceMapTest, TestGetMaxCap)
{
{
auto smap = SpaceMap::createSpaceMap(test_type, 0, 100);
ASSERT_TRUE(smap->markUsed(50, 10));
ASSERT_TRUE(smap->markUsed(80, 10));

ASSERT_EQ(smap->getMaxCapability(), 50);
}

{
auto smap = SpaceMap::createSpaceMap(test_type, 0, 100);
ASSERT_TRUE(smap->markUsed(0, 100));

ASSERT_EQ(smap->getMaxCapability(), 0);
}
}

INSTANTIATE_TEST_CASE_P(
Type,
SpaceMapTest,
Expand Down