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

User/qkrorlqr/issue 95 compaction garbage blocks #731

Closed
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .github/workflows/build_and_test_on_demand.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ jobs:
with:
submodules: true
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: ${{ contains(github.event.pull_request.labels.*.name, 'rebase') && 0 || 1 }}
- name: Rebase PR
if: ${{ github.event.pull_request.head.sha != '' && contains(github.event.pull_request.labels.*.name, 'rebase') }}
shell: bash
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/build_and_test_on_demand_cmake.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ jobs:
with:
submodules: true
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: ${{ contains(github.event.pull_request.labels.*.name, 'rebase') && 0 || 1 }}
- name: Rebase PR
if: ${{ github.event.pull_request.head.sha != '' && contains(github.event.pull_request.labels.*.name, 'rebase') }}
shell: bash
Expand Down
6 changes: 3 additions & 3 deletions cloud/filestore/libs/storage/tablet/rebase_logic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ namespace NCloud::NFileStore::NStorage {

////////////////////////////////////////////////////////////////////////////////

TRebaseResult RebaseMixedBlocks(
TRebaseResult RebaseBlocks(
TVector<TBlock>& blocks,
ui64 lastCommitId,
TFindCheckpoint findCheckpoint,
TFindBlock findBlock)
const TFindCheckpoint& findCheckpoint,
const TFindBlock& findBlock)
{
TRebaseResult result;

Expand Down
6 changes: 3 additions & 3 deletions cloud/filestore/libs/storage/tablet/rebase_logic.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ using TFindBlock = std::function<bool(ui64 nodeId, ui32 blockIndex)>;

////////////////////////////////////////////////////////////////////////////////

TRebaseResult RebaseMixedBlocks(
TRebaseResult RebaseBlocks(
TVector<TBlock>& blocks,
ui64 lastCommitId,
TFindCheckpoint findCheckpoint,
TFindBlock findBlock);
const TFindCheckpoint& findCheckpoint,
const TFindBlock& findBlock);

} // namespace NCloud::NFileStore::NStorage
3 changes: 3 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "checkpoint.h"
#include "helpers.h"
#include "rebase_logic.h"
#include "session.h"

#include <cloud/filestore/libs/storage/model/channel_data_kind.h>
Expand Down Expand Up @@ -741,6 +742,8 @@ FILESTORE_DUPCACHE_REQUESTS(FILESTORE_DECLARE_DUPCACHE)
const TPartialBlobId& blobId,
const TVector<TBlock>& blocks);

TRebaseResult RebaseMixedBlocks(TVector<TBlock>& blocks) const;

//
// Garbage
//
Expand Down
58 changes: 26 additions & 32 deletions cloud/filestore/libs/storage/tablet/tablet_state_data.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include "tablet_state_impl.h"

#include "profile_log_events.h"
#include "rebase_logic.h"

#include <cloud/filestore/libs/storage/model/utils.h>
#include <cloud/filestore/libs/storage/tablet/model/block.h>
Expand Down Expand Up @@ -580,21 +579,7 @@ bool TIndexTabletState::WriteMixedBlocks(
Impl->MixedBlocks.ApplyDeletionMarkers(GetRangeIdHasher(), blocks);
}

auto rebaseResult = RebaseMixedBlocks(
blocks,
GetCurrentCommitId(),
[=] (ui64 nodeId, ui64 commitId) {
return Impl->Checkpoints.FindCheckpoint(nodeId, commitId);
},
[=] (ui64 nodeId, ui32 blockIndex) {
return IntersectsWithFresh(
Impl->FreshBytes,
Impl->FreshBlocks,
GetBlockSize(),
nodeId,
blockIndex
);
});
auto rebaseResult = RebaseMixedBlocks(blocks);

if (!rebaseResult.LiveBlocks) {
AddGarbageBlob(db, blobId);
Expand Down Expand Up @@ -676,9 +661,32 @@ void TIndexTabletState::DeleteMixedBlocks(
DecrementMixedBlocksCount(db, blocks.size());
}

TRebaseResult TIndexTabletState::RebaseMixedBlocks(TVector<TBlock>& blocks) const
{
return RebaseBlocks(
blocks,
GetCurrentCommitId(),
[=] (ui64 nodeId, ui64 commitId) {
return Impl->Checkpoints.FindCheckpoint(nodeId, commitId);
},
[=] (ui64 nodeId, ui32 blockIndex) {
return IntersectsWithFresh(
Impl->FreshBytes,
Impl->FreshBlocks,
GetBlockSize(),
nodeId,
blockIndex
);
});
}

TVector<TMixedBlobMeta> TIndexTabletState::GetBlobsForCompaction(ui32 rangeId) const
{
return Impl->MixedBlocks.GetBlobsForCompaction(rangeId);
auto blobs = Impl->MixedBlocks.GetBlobsForCompaction(rangeId);
for (auto& blob: blobs) {
RebaseMixedBlocks(blob.Blocks);
}
return blobs;
}

TMixedBlobMeta TIndexTabletState::FindBlob(ui32 rangeId, TPartialBlobId blobId) const
Expand Down Expand Up @@ -808,21 +816,7 @@ void TIndexTabletState::RewriteMixedBlocks(

Impl->MixedBlocks.ApplyDeletionMarkers(GetRangeIdHasher(), blob.Blocks);

auto rebaseResult = RebaseMixedBlocks(
blob.Blocks,
GetCurrentCommitId(),
[=] (ui64 nodeId, ui64 commitId) {
return Impl->Checkpoints.FindCheckpoint(nodeId, commitId);
},
[=] (ui64 nodeId, ui32 blockIndex) {
return IntersectsWithFresh(
Impl->FreshBytes,
Impl->FreshBlocks,
GetBlockSize(),
nodeId,
blockIndex
);
});
auto rebaseResult = RebaseMixedBlocks(blob.Blocks);

if (!rebaseResult.LiveBlocks) {
DeleteMixedBlocks(db, blob.BlobId, blob.Blocks);
Expand Down
14 changes: 8 additions & 6 deletions cloud/filestore/libs/storage/tablet/tablet_ut_data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,9 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
const auto& stats = response->Record.GetStats();
UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 0);
UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlobsCount(), 0);
UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), 2 * block);
// still 1 block - Compaction doesn't move unneeded blocks to its
// dst blobs
UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), block);
}

id = CreateNode(tablet, TCreateNodeArgs::File(RootNodeId, "test"));
Expand All @@ -1001,7 +1003,7 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
// GarbageQueueSize remains the same since FlushBytes doesn't
// generate any new blobs after UnlinkNode (leads to Truncate to
// zero size)
UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), 2 * block);
UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), block);
}

tablet.CollectGarbage();
Expand Down Expand Up @@ -1063,20 +1065,20 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
{
auto response = tablet.GetStorageStats();
const auto& stats = response->Record.GetStats();
UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 2);
UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 1);
UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlobsCount(), 1);
UNIT_ASSERT_VALUES_EQUAL(
stats.GetGarbageQueueSize(),
2 * block + 2 * block
); // new: 1 (x 2 blocks), garbage: 2 (x block)
1 * block + 2 * block
); // new: 1 (x block), garbage: 2 (x block)
}

tablet.CollectGarbage();

{
auto response = tablet.GetStorageStats();
const auto& stats = response->Record.GetStats();
UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 2);
UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 1);
UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlobsCount(), 1);
UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), 0); // new: 0, garbage: 0
}
Expand Down