Resolve some review comments

This commit is contained in:
kssenii 2023-01-06 16:10:00 +01:00
parent 2a030c1dc0
commit 8fde606768
9 changed files with 21 additions and 21 deletions

View File

@ -105,20 +105,20 @@ DiskSelectorPtr DiskSelector::updateFromConfig(
else
writeString("Disks ", warning);
int index = 0;
int num_disks_removed_from_config = 0;
for (const auto & [name, disk] : old_disks_minus_new_disks)
{
/// Custom disks are not present in config.
if (disk->isCustomDisk())
continue;
if (index++ > 0)
if (num_disks_removed_from_config++ > 0)
writeString(", ", warning);
writeBackQuotedString(name, warning);
}
if (index > 0)
if (num_disks_removed_from_config > 0)
{
writeString(" disappeared from configuration, this change will be applied after restart of ClickHouse", warning);
LOG_WARNING(&Poco::Logger::get("DiskSelector"), fmt::runtime(warning.str()));

View File

@ -24,7 +24,7 @@ void registerDiskAzureBlobStorage(DiskFactory & factory, bool global_skip_access
ContextPtr context,
const DisksMap & /*map*/)
{
auto [metadata_path, metadata_disk] = prepareForLocalMetadata(name, config, config_prefix, context->getPath());
auto [metadata_path, metadata_disk] = prepareForLocalMetadata(name, config, config_prefix, context);
ObjectStoragePtr azure_object_storage = std::make_unique<AzureObjectStorage>(
name,

View File

@ -11,19 +11,19 @@ static String getDiskMetadataPath(
const String & name,
const Poco::Util::AbstractConfiguration & config,
const String & config_prefix,
const String & root_disks_path)
ContextPtr context)
{
return config.getString(config_prefix + ".metadata_path", fs::path(root_disks_path) / "disks" / name / "");
return config.getString(config_prefix + ".metadata_path", fs::path(context->getPath()) / "disks" / name / "");
}
std::pair<String, DiskPtr> prepareForLocalMetadata(
const String & name,
const Poco::Util::AbstractConfiguration & config,
const String & config_prefix,
const String & root_disks_path)
ContextPtr context)
{
/// where the metadata files are stored locally
auto metadata_path = getDiskMetadataPath(name, config, config_prefix, root_disks_path);
auto metadata_path = getDiskMetadataPath(name, config, config_prefix, context);
fs::create_directories(metadata_path);
auto metadata_disk = std::make_shared<DiskLocal>(name + "-metadata", metadata_path, 0);
return std::make_pair(metadata_path, metadata_disk);

View File

@ -16,7 +16,7 @@ std::pair<String, DiskPtr> prepareForLocalMetadata(
const String & name,
const Poco::Util::AbstractConfiguration & config,
const String & config_prefix,
const String & root_disks_path);
ContextPtr context);
bool isFileWithPersistentCache(const String & path);

View File

@ -41,7 +41,7 @@ void registerDiskHDFS(DiskFactory & factory, bool global_skip_access_check)
/// FIXME Cache currently unsupported :(
ObjectStoragePtr hdfs_storage = std::make_unique<HDFSObjectStorage>(uri, std::move(settings), config);
auto [_, metadata_disk] = prepareForLocalMetadata(name, config, config_prefix, context->getPath());
auto [_, metadata_disk] = prepareForLocalMetadata(name, config, config_prefix, context);
auto metadata_storage = std::make_shared<MetadataStorageFromDisk>(metadata_disk, uri);
uint64_t copy_thread_pool_size = config.getUInt(config_prefix + ".thread_pool_size", 16);

View File

@ -131,7 +131,7 @@ void registerDiskS3(DiskFactory & factory, bool global_skip_access_check)
else
{
s3_storage = std::make_shared<S3ObjectStorage>(std::move(client), std::move(settings), uri.version_id, s3_capabilities, uri.bucket, uri.endpoint);
auto [metadata_path, metadata_disk] = prepareForLocalMetadata(name, config, config_prefix, context->getPath());
auto [metadata_path, metadata_disk] = prepareForLocalMetadata(name, config, config_prefix, context);
metadata_storage = std::make_shared<MetadataStorageFromDisk>(metadata_disk, uri.key);
}

View File

@ -2920,19 +2920,19 @@ StoragePolicyPtr Context::getStoragePolicy(const String & name) const
return policy_selector->get(name);
}
StoragePolicyPtr Context::getStoragePolicyFromDisk(const String & name) const
StoragePolicyPtr Context::getStoragePolicyFromDisk(const String & disk_name) const
{
std::lock_guard lock(shared->storage_policies_mutex);
const std::string storage_policy_name = StoragePolicySelector::TMP_STORAGE_POLICY_PREFIX + name;
const std::string storage_policy_name = StoragePolicySelector::TMP_STORAGE_POLICY_PREFIX + disk_name;
auto storage_policy_selector = getStoragePolicySelector(lock);
StoragePolicyPtr storage_policy = storage_policy_selector->tryGet(storage_policy_name);
if (!storage_policy)
{
auto disk_selector = getDiskSelector(lock);
auto disk = disk_selector->get(name);
auto volume = std::make_shared<SingleDiskVolume>("_volume_" + name, disk);
auto disk = disk_selector->get(disk_name);
auto volume = std::make_shared<SingleDiskVolume>("_volume_" + disk_name, disk);
static const auto move_factor_for_single_disk_volume = 0.0;
storage_policy = std::make_shared<StoragePolicy>(storage_policy_name, Volumes{volume}, move_factor_for_single_disk_volume);

View File

@ -962,7 +962,7 @@ public:
/// Provides storage politics schemes
StoragePolicyPtr getStoragePolicy(const String & name) const;
StoragePolicyPtr getStoragePolicyFromDisk(const String & name) const;
StoragePolicyPtr getStoragePolicyFromDisk(const String & disk_name) const;
/// Get the server uptime in seconds.
double getUptimeSeconds() const;

View File

@ -90,10 +90,10 @@ MergeTreeReadTaskPtr MergeTreeReadPool::getTask(size_t min_marks_to_read, size_t
auto & marks_in_part = thread_tasks.sum_marks_in_parts.back();
size_t need_marks;
// if (is_part_on_remote_disk[part_idx]) /// For better performance with remote disks
// need_marks = marks_in_part;
// else /// Get whole part to read if it is small enough.
need_marks = std::min(marks_in_part, min_marks_to_read);
if (is_part_on_remote_disk[part_idx]) /// For better performance with remote disks
need_marks = marks_in_part;
else /// Get whole part to read if it is small enough.
need_marks = std::min(marks_in_part, min_marks_to_read);
/// Do not leave too little rows in part for next time.
if (marks_in_part > need_marks &&
@ -203,7 +203,7 @@ std::vector<size_t> MergeTreeReadPool::fillPerPartInfo(const RangesInDataParts &
const auto & part = parts[i];
bool part_on_remote_disk = part.data_part->isStoredOnRemoteDisk();
is_part_on_remote_disk[i] = part_on_remote_disk;
// do_not_steal_tasks |= part_on_remote_disk;
do_not_steal_tasks |= part_on_remote_disk;
/// Read marks for every data part.
size_t sum_marks = 0;