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

Backport 8549 to 19.16 [Do not allow to merge data moving it against storage policy volume order] #9496

Merged
merged 1 commit into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions dbms/src/Storages/MergeTree/MergeTreeData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3095,13 +3095,13 @@ MergeTreeData::MutableDataPartsVector MergeTreeData::tryLoadPartsToAttach(const
return loaded_parts;
}

DiskSpace::ReservationPtr MergeTreeData::reserveSpace(UInt64 expected_size)
DiskSpace::ReservationPtr MergeTreeData::reserveSpace(UInt64 expected_size, size_t min_volume_index)
{
constexpr UInt64 RESERVATION_MIN_ESTIMATION_SIZE = 1u * 1024u * 1024u; /// 1MB

expected_size = std::max(RESERVATION_MIN_ESTIMATION_SIZE, expected_size);

auto reservation = storage_policy->reserve(expected_size);
auto reservation = storage_policy->reserve(expected_size, min_volume_index);
if (reservation)
return reservation;

Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Storages/MergeTree/MergeTreeData.h
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ class MergeTreeData : public IStorage
Strings getDataPaths() const override;

/// Reserves space at least 1MB
DiskSpace::ReservationPtr reserveSpace(UInt64 expected_size);
DiskSpace::ReservationPtr reserveSpace(UInt64 expected_size, size_t min_volume_index = 0);

/// Choose disk with max available free space
/// Reserves 0 bytes
Expand Down
8 changes: 7 additions & 1 deletion dbms/src/Storages/StorageMergeTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,13 @@ struct CurrentlyMergingPartsTagger
if (is_mutation)
reserved_space = future_part_.parts[0]->disk->reserve(total_size);
else
reserved_space = storage.reserveSpace(total_size);
{
size_t max_volume_index = 0;
for (auto & part_ptr : future_part_.parts)
max_volume_index = std::max(max_volume_index, storage.getStoragePolicy()->getVolumeIndexByDisk(part_ptr->disk));

reserved_space = storage.reserveSpace(total_size, max_volume_index);
}
if (!reserved_space)
{
if (is_mutation)
Expand Down
6 changes: 5 additions & 1 deletion dbms/src/Storages/StorageReplicatedMergeTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1005,8 +1005,12 @@ bool StorageReplicatedMergeTree::tryExecuteMerge(const LogEntry & entry)
/// Start to make the main work
size_t estimated_space_for_merge = MergeTreeDataMergerMutator::estimateNeededDiskSpace(parts);

size_t max_volume_index = 0;
for (auto & part_ptr : parts)
max_volume_index = std::max(max_volume_index, getStoragePolicy()->getVolumeIndexByDisk(part_ptr->disk));

/// Can throw an exception.
DiskSpace::ReservationPtr reserved_space = reserveSpace(estimated_space_for_merge);
DiskSpace::ReservationPtr reserved_space = reserveSpace(estimated_space_for_merge, max_volume_index);

auto table_lock = lockStructureForShare(false, RWLockImpl::NO_QUERY);

Expand Down