Fixed implementation of DiskLocal::moveTo

This commit is contained in:
Alexander Burmak 2019-11-28 22:11:49 +03:00
parent 910ceb67b3
commit 9c60149042
10 changed files with 112 additions and 61 deletions

View File

@ -87,7 +87,7 @@ PenaltyBreakComment: 300
PenaltyBreakFirstLessLess: 120
PenaltyBreakString: 1000
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 1000
PenaltyReturnTypeOnItsOwnLine: 60
SpaceAfterCStyleCast: false
SpaceBeforeAssignmentOperators: true
SpaceBeforeParens: ControlStatements

View File

@ -14,13 +14,13 @@ DiskFactory & DiskFactory::instance()
return factory;
}
void DiskFactory::registerDisk(const String & disk_type, DB::DiskFactory::Creator creator)
void DiskFactory::registerDiskType(const String & disk_type, DB::DiskFactory::Creator creator)
{
if (!registry.emplace(disk_type, creator).second)
throw Exception("DiskFactory: the disk type '" + disk_type + "' is not unique", ErrorCodes::LOGICAL_ERROR);
}
DiskPtr DiskFactory::get(
DiskPtr DiskFactory::create(
const String & name,
const Poco::Util::AbstractConfiguration & config,
const String & config_prefix,

View File

@ -13,20 +13,25 @@ namespace DB
{
class Context;
/**
* Disk factory. Responsible for creating new disk objects.
*/
class DiskFactory final : private boost::noncopyable
{
public:
using Creator = std::function<DiskPtr(
const String & name, const Poco::Util::AbstractConfiguration & config,
const String & name,
const Poco::Util::AbstractConfiguration & config,
const String & config_prefix,
const Context & context)>;
static DiskFactory & instance();
void registerDisk(const String & disk_type, Creator creator);
void registerDiskType(const String & disk_type, Creator creator);
DiskPtr get(
const String & name, const Poco::Util::AbstractConfiguration & config,
DiskPtr create(
const String & name,
const Poco::Util::AbstractConfiguration & config,
const String & config_prefix,
const Context & context) const;

View File

@ -1,9 +1,9 @@
#include "DiskLocal.h"
#include "DiskFactory.h"
#include <Interpreters/Context.h>
#include <Common/filesystemHelpers.h>
#include <Common/quoteString.h>
#include <Interpreters/Context.h>
namespace DB
@ -76,6 +76,52 @@ DiskFilePtr DiskLocal::file(const String & path_) const
}
DiskLocalFile::DiskLocalFile(const DiskPtr & disk_ptr_, const String & rel_path_)
: IDiskFile(disk_ptr_, rel_path_), file(disk_ptr->getPath() + rel_path)
{
}
bool DiskLocalFile::exists() const
{
return file.exists();
}
bool DiskLocalFile::isDirectory() const
{
return file.isDirectory();
}
void DiskLocalFile::createDirectory()
{
file.createDirectory();
}
void DiskLocalFile::createDirectories()
{
file.createDirectories();
}
void DiskLocalFile::moveTo(const String & new_path)
{
file.renameTo(disk_ptr->getPath() + new_path);
}
void DiskLocalFile::copyTo(const String & new_path)
{
file.copyTo(disk_ptr->getPath() + new_path);
}
std::unique_ptr<ReadBuffer> DiskLocalFile::read() const
{
return std::make_unique<ReadBufferFromFile>(file.path());
}
std::unique_ptr<WriteBuffer> DiskLocalFile::write()
{
return std::make_unique<WriteBufferFromFile>(file.path());
}
DiskDirectoryIteratorImplPtr DiskLocalFile::iterateDirectory()
{
return std::make_unique<DiskLocalDirectoryIterator>(shared_from_this());
@ -188,7 +234,7 @@ void registerDiskLocal(DiskFactory & factory)
return std::make_shared<const DiskLocal>(name, path, keep_free_space_bytes);
};
factory.registerDisk("local", creator);
factory.registerDiskType("local", creator);
}
}

View File

@ -65,26 +65,23 @@ using DiskLocalPtr = std::shared_ptr<const DiskLocal>;
class DiskLocalFile : public IDiskFile
{
public:
DiskLocalFile(const DiskPtr & disk_ptr_, const String & rel_path_)
: IDiskFile(disk_ptr_, rel_path_), file(disk_ptr->getPath() + rel_path)
{
}
DiskLocalFile(const DiskPtr & disk_ptr_, const String & rel_path_);
bool exists() const override { return file.exists(); }
bool exists() const override;
bool isDirectory() const override { return file.isDirectory(); }
bool isDirectory() const override;
void createDirectory() override { file.createDirectory(); }
void createDirectory() override;
void createDirectories() override { file.createDirectories(); }
void createDirectories() override;
void moveTo(const String & new_path) override { file.renameTo(new_path); }
void moveTo(const String & new_path) override;
void copyTo(const String & new_path) override { file.copyTo(new_path); }
void copyTo(const String & new_path) override;
std::unique_ptr<ReadBuffer> read() const override { return std::make_unique<ReadBufferFromFile>(file.path()); }
std::unique_ptr<ReadBuffer> read() const override;
std::unique_ptr<WriteBuffer> write() override { return std::make_unique<WriteBufferFromFile>(file.path()); }
std::unique_ptr<WriteBuffer> write() override;
private:
DiskDirectoryIteratorImplPtr iterateDirectory() override;

View File

@ -1,9 +1,10 @@
#include "DiskSpaceMonitor.h"
#include "DiskFactory.h"
#include "DiskLocal.h"
#include <Interpreters/Context.h>
#include <Common/escapeForFileName.h>
#include <Common/quoteString.h>
#include <Interpreters/Context.h>
#include <set>
#include <Poco/File.h>
@ -11,7 +12,6 @@
namespace DB
{
DiskSelector::DiskSelector(const Poco::Util::AbstractConfiguration & config, const String & config_prefix, const Context & context)
{
Poco::Util::AbstractConfiguration::Keys keys;
@ -24,15 +24,14 @@ DiskSelector::DiskSelector(const Poco::Util::AbstractConfiguration & config, con
for (const auto & disk_name : keys)
{
if (!std::all_of(disk_name.begin(), disk_name.end(), isWordCharASCII))
throw Exception("Disk name can contain only alphanumeric and '_' (" + disk_name + ")",
ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
throw Exception("Disk name can contain only alphanumeric and '_' (" + disk_name + ")", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
if (disk_name == default_disk_name)
has_default_disk = true;
auto disk_config_prefix = config_prefix + "." + disk_name;
disks.emplace(disk_name, factory.get(disk_name, config, disk_config_prefix, context));
disks.emplace(disk_name, factory.create(disk_name, config, disk_config_prefix, context));
}
if (!has_default_disk)
disks.emplace(default_disk_name, std::make_shared<const DiskLocal>(default_disk_name, context.getPath(), 0));
@ -75,8 +74,9 @@ Volume::Volume(
auto has_max_bytes = config.has(config_prefix + ".max_data_part_size_bytes");
auto has_max_ratio = config.has(config_prefix + ".max_data_part_size_ratio");
if (has_max_bytes && has_max_ratio)
throw Exception("Only one of 'max_data_part_size_bytes' and 'max_data_part_size_ratio' should be specified.",
ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
throw Exception(
"Only one of 'max_data_part_size_bytes' and 'max_data_part_size_ratio' should be specified.",
ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
if (has_max_bytes)
{
@ -86,8 +86,7 @@ Volume::Volume(
{
auto ratio = config.getDouble(config_prefix + ".max_data_part_size_ratio");
if (ratio < 0)
throw Exception("'max_data_part_size_ratio' have to be not less then 0.",
ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
throw Exception("'max_data_part_size_ratio' have to be not less then 0.", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
UInt64 sum_size = 0;
std::vector<UInt64> sizes;
for (const auto & disk : disks)
@ -98,16 +97,18 @@ Volume::Volume(
max_data_part_size = static_cast<decltype(max_data_part_size)>(sum_size * ratio / disks.size());
for (size_t i = 0; i < disks.size(); ++i)
if (sizes[i] < max_data_part_size)
LOG_WARNING(logger, "Disk " << backQuote(disks[i]->getName()) << " on volume " << backQuote(config_prefix) <<
" have not enough space (" << formatReadableSizeWithBinarySuffix(sizes[i]) <<
") for containing part the size of max_data_part_size (" <<
formatReadableSizeWithBinarySuffix(max_data_part_size) << ")");
LOG_WARNING(
logger,
"Disk " << backQuote(disks[i]->getName()) << " on volume " << backQuote(config_prefix) << " have not enough space ("
<< formatReadableSizeWithBinarySuffix(sizes[i]) << ") for containing part the size of max_data_part_size ("
<< formatReadableSizeWithBinarySuffix(max_data_part_size) << ")");
}
constexpr UInt64 MIN_PART_SIZE = 8u * 1024u * 1024u;
if (max_data_part_size != 0 && max_data_part_size < MIN_PART_SIZE)
LOG_WARNING(logger, "Volume " << backQuote(name) << " max_data_part_size is too low ("
<< formatReadableSizeWithBinarySuffix(max_data_part_size) << " < "
<< formatReadableSizeWithBinarySuffix(MIN_PART_SIZE) << ")");
LOG_WARNING(
logger,
"Volume " << backQuote(name) << " max_data_part_size is too low (" << formatReadableSizeWithBinarySuffix(max_data_part_size)
<< " < " << formatReadableSizeWithBinarySuffix(MIN_PART_SIZE) << ")");
}
@ -158,7 +159,8 @@ StoragePolicy::StoragePolicy(
for (const auto & attr_name : keys)
{
if (!std::all_of(attr_name.begin(), attr_name.end(), isWordCharASCII))
throw Exception("Volume name can contain only alphanumeric and '_' (" + attr_name + ")", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
throw Exception(
"Volume name can contain only alphanumeric and '_' (" + attr_name + ")", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
volumes.push_back(std::make_shared<Volume>(attr_name, config, volumes_prefix + "." + attr_name, disks));
if (volumes_names.find(attr_name) != volumes_names.end())
throw Exception("Volumes names must be unique (" + attr_name + " duplicated)", ErrorCodes::UNKNOWN_POLICY);
@ -175,7 +177,8 @@ StoragePolicy::StoragePolicy(
for (const auto & disk : volume->disks)
{
if (disk_names.find(disk->getName()) != disk_names.end())
throw Exception("Duplicate disk '" + disk->getName() + "' in storage policy '" + name + "'", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
throw Exception(
"Duplicate disk '" + disk->getName() + "' in storage policy '" + name + "'", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
disk_names.insert(disk->getName());
}
@ -183,23 +186,18 @@ StoragePolicy::StoragePolicy(
move_factor = config.getDouble(config_prefix + ".move_factor", 0.1);
if (move_factor > 1)
throw Exception("Disk move factor have to be in [0., 1.] interval, but set to " + toString(move_factor),
ErrorCodes::LOGICAL_ERROR);
throw Exception("Disk move factor have to be in [0., 1.] interval, but set to " + toString(move_factor), ErrorCodes::LOGICAL_ERROR);
}
StoragePolicy::StoragePolicy(String name_, Volumes volumes_, double move_factor_)
: volumes(std::move(volumes_))
, name(std::move(name_))
, move_factor(move_factor_)
: volumes(std::move(volumes_)), name(std::move(name_)), move_factor(move_factor_)
{
if (volumes.empty())
throw Exception("StoragePolicy must contain at least one Volume.", ErrorCodes::UNKNOWN_POLICY);
if (move_factor > 1)
throw Exception("Disk move factor have to be in [0., 1.] interval, but set to " + toString(move_factor),
ErrorCodes::LOGICAL_ERROR);
throw Exception("Disk move factor have to be in [0., 1.] interval, but set to " + toString(move_factor), ErrorCodes::LOGICAL_ERROR);
for (size_t i = 0; i < volumes.size(); ++i)
{
@ -316,8 +314,8 @@ StoragePolicySelector::StoragePolicySelector(
for (const auto & name : keys)
{
if (!std::all_of(name.begin(), name.end(), isWordCharASCII))
throw Exception("StoragePolicy name can contain only alphanumeric and '_' (" + name + ")",
ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
throw Exception(
"StoragePolicy name can contain only alphanumeric and '_' (" + name + ")", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG);
policies.emplace(name, std::make_shared<StoragePolicy>(name, config, config_prefix + "." + name, disks));
LOG_INFO(&Logger::get("StoragePolicySelector"), "Storage policy " << backQuote(name) << " loaded");
@ -330,10 +328,7 @@ StoragePolicySelector::StoragePolicySelector(
/// Add default policy if it's not specified explicetly
if (policies.find(default_storage_policy_name) == policies.end())
{
auto default_volume = std::make_shared<Volume>(
default_volume_name,
std::vector<DiskPtr>{disks[default_disk_name]},
0);
auto default_volume = std::make_shared<Volume>(default_volume_name, std::vector<DiskPtr>{disks[default_disk_name]}, 0);
auto default_policy = std::make_shared<StoragePolicy>(default_storage_policy_name, Volumes{default_volume}, 0.0);
policies.emplace(default_storage_policy_name, default_policy);

View File

@ -1,11 +1,11 @@
#pragma once
#include <common/logger_useful.h>
#include <Disks/IDisk.h>
#include <IO/WriteHelpers.h>
#include <Common/CurrentMetrics.h>
#include <Common/Exception.h>
#include <Common/formatReadable.h>
#include <Disks/IDisk.h>
#include <IO/WriteHelpers.h>
#include <common/logger_useful.h>
#include <memory>
#include <mutex>
@ -38,7 +38,7 @@ public:
/// Get disk by name
const DiskPtr & operator[](const String & name) const;
/// Get all disks name
/// Get all disks with names
const auto & getDisksMap() const { return disks; }
private:
@ -62,7 +62,10 @@ public:
}
Volume(
String name_, const Poco::Util::AbstractConfiguration & config, const String & config_prefix, const DiskSelector & disk_selector);
String name_,
const Poco::Util::AbstractConfiguration & config,
const String & config_prefix,
const DiskSelector & disk_selector);
/// Uses Round-robin to choose disk for reservation.
/// Returns valid reservation or nullptr if there is no space left on any disk.

View File

@ -1,8 +1,8 @@
#pragma once
#include <Core/Types.h>
#include <Common/CurrentMetrics.h>
#include <Common/Exception.h>
#include <Core/Types.h>
#include <memory>
#include <utility>
@ -44,6 +44,13 @@ public:
using SpacePtr = std::shared_ptr<const Space>;
/**
* A unit of storage persisting data and metadata.
* Abstract underlying storage technology.
* Responsible for:
* - file management;
* - space accounting and reservation.
*/
class IDisk : public Space
{
public:

View File

@ -2,7 +2,6 @@
namespace DB
{
void registerDiskLocal(DiskFactory & factory);
void registerDisks()

View File

@ -2,7 +2,6 @@
namespace DB
{
void registerDisks();
}