This commit is contained in:
Alexander Tokmakov 2019-07-26 23:04:45 +03:00
parent 3ba26aba43
commit 0fc47fbbe4
12 changed files with 69 additions and 63 deletions

View File

@ -87,6 +87,8 @@ namespace ErrorCodes
extern const int CANNOT_MUNMAP;
extern const int CANNOT_MREMAP;
extern const int BAD_TTL_EXPRESSION;
extern const int INCORRECT_FILE_NAME;
extern const int BAD_DATA_PART_NAME;
}
@ -2583,11 +2585,21 @@ MergeTreeData::getDetachedParts() const
res.emplace_back();
auto & part = res.back();
DetachedPartInfo::tryParseDetachedPartName(dir_name, &part, format_version);
DetachedPartInfo::tryParseDetachedPartName(dir_name, part, format_version);
}
return res;
}
void MergeTreeData::validateDetachedPartName(const String & name) const
{
if (name.find('/') != std::string::npos || name == "." || name == "..")
throw DB::Exception("Invalid part name", ErrorCodes::INCORRECT_FILE_NAME);
Poco::File detached_part_dir(full_path + "detached/" + name);
if (!detached_part_dir.exists())
throw DB::Exception("Detached part \"" + name + "\" not found" , ErrorCodes::BAD_DATA_PART_NAME);
}
MergeTreeData::DataParts MergeTreeData::getDataParts(const DataPartStates & affordable_states) const
{
DataParts res;

View File

@ -389,6 +389,8 @@ public:
/// Returns all detached parts
std::vector<DetachedPartInfo> getDetachedParts() const;
void validateDetachedPartName(const String & name) const;
/// Returns Committed parts
DataParts getDataParts() const;
DataPartsVector getDataPartsVector() const;

View File

@ -356,16 +356,18 @@ void MergeTreeDataPart::remove(bool force_recursive /*= false*/) const
* - rename directory to temporary name;
* - remove it recursive.
*
* For temporary name we use "delete_tmp_" prefix.
* For temporary name we use "detached/deleting_" prefix.
*
* NOTE: We cannot use "tmp_delete_" prefix, because there is a second thread,
* NOTE: We cannot use "tmp_*" prefix, because there is a second thread,
* that calls "clearOldTemporaryDirectories" and removes all directories, that begin with "tmp_" and are old enough.
* But when we removing data part, it can be old enough. And rename doesn't change mtime.
* And a race condition can happen that will lead to "File not found" error here.
* We move directory to detached/, because if an attempt to remove directory after renaming failed for some reason
* there would be no way to remove directory from storage.full_path (except manually).
*/
String from = storage.full_path + relative_path;
String to = storage.full_path + "delete_tmp_" + name;
String to = storage.full_path + getRelativePathForDetachedPart("deleting_");
Poco::File from_dir{from};
Poco::File to_dir{to};

View File

@ -188,35 +188,29 @@ String MergeTreePartInfo::getPartNameV0(DayNum left_date, DayNum right_date) con
return wb.str();
}
bool DetachedPartInfo::tryParseDetachedPartName(const String & dir_name, DetachedPartInfo * part_info,
bool DetachedPartInfo::tryParseDetachedPartName(const String & dir_name, DetachedPartInfo & part_info,
MergeTreeDataFormatVersion format_version)
{
part_info.dir_name = dir_name;
/// First, try to parse as <part_name>.
if (MergeTreePartInfo::tryParsePartName(dir_name, part_info, format_version))
return part_info->valid_name = true;
if (MergeTreePartInfo::tryParsePartName(dir_name, &part_info, format_version))
return part_info.valid_name = true;
/// Next, as <prefix>_<partname>. Use entire name as prefix if it fails.
part_info->prefix = dir_name;
part_info.prefix = dir_name;
const auto first_separator = dir_name.find_first_of('_');
if (first_separator == String::npos)
return part_info->valid_name = false;
return part_info.valid_name = false;
// TODO what if <prefix> contains '_'?
const auto part_name = dir_name.substr(first_separator + 1,
dir_name.size() - first_separator - 1);
if (!MergeTreePartInfo::tryParsePartName(part_name, part_info, format_version))
return part_info->valid_name = false;
if (!MergeTreePartInfo::tryParsePartName(part_name, &part_info, format_version))
return part_info.valid_name = false;
part_info->prefix = dir_name.substr(0, first_separator);
return part_info->valid_name = true;
part_info.prefix = dir_name.substr(0, first_separator);
return part_info.valid_name = true;
}
String DetachedPartInfo::fullDirName() const
{
if (!valid_name)
return prefix;
if (prefix.empty())
return getPartName();
return prefix + "_" + getPartName();
}
}

View File

@ -92,15 +92,14 @@ struct MergeTreePartInfo
/// addition to the above fields.
struct DetachedPartInfo : public MergeTreePartInfo
{
/// Suddenly, name of detached part may contain suffix (such as _tryN), which is ignored by MergeTreePartInfo::tryParsePartName(...)
String dir_name;
String prefix;
/// If false, prefix contains full directory name and MergeTreePartInfo may be in invalid state
/// (directory name was not successfully parsed).
/// If false, MergeTreePartInfo is in invalid state (directory name was not successfully parsed).
bool valid_name;
String fullDirName() const;
static bool tryParseDetachedPartName(const String & dir_name, DetachedPartInfo * part_info, MergeTreeDataFormatVersion format_version);
static bool tryParseDetachedPartName(const String & dir_name, DetachedPartInfo & part_info, MergeTreeDataFormatVersion format_version);
};
}

View File

@ -36,7 +36,6 @@ namespace ErrorCodes
extern const int INCORRECT_FILE_NAME;
extern const int CANNOT_ASSIGN_OPTIMIZE;
extern const int INCOMPATIBLE_COLUMNS;
extern const int BAD_DATA_PART_NAME;
}
namespace ActionLocks
@ -1003,7 +1002,7 @@ void StorageMergeTree::dropDetached(const ASTPtr & partition, bool part, const C
validateDetachedPartName(part_id);
DetachedPartInfo info;
DetachedPartInfo::tryParseDetachedPartName(part_id, &info, format_version);
DetachedPartInfo::tryParseDetachedPartName(part_id, info, format_version);
MergeTreeDataPart detached_part(*this, part_id, info);
detached_part.relative_path = "detached/" + part_id;
@ -1038,7 +1037,8 @@ void StorageMergeTree::attachPartition(const ASTPtr & partition, bool attach_par
{
const String & name = it.name();
MergeTreePartInfo part_info;
/// Parts with prefix in name (e.g. attaching_1_3_3_0, delete_tmp_1_3_3_0) will be ignored
/// Parts with prefix in name (e.g. attaching_1_3_3_0, deleting_1_3_3_0) will be ignored
// TODO what if name contains "_tryN" suffix?
if (!MergeTreePartInfo::tryParsePartName(name, &part_info, format_version)
|| part_info.partition_id != partition_id)
{
@ -1163,15 +1163,6 @@ void StorageMergeTree::replacePartitionFrom(const StoragePtr & source_table, con
}
}
void StorageMergeTree::validateDetachedPartName(const String & name) const
{
if (name.find('/') != std::string::npos || name == "." || name == "..")
throw DB::Exception("Invalid part name", ErrorCodes::INCORRECT_FILE_NAME);
Poco::File detached_part_dir(full_path + "detached/" + name);
if (!detached_part_dir.exists())
throw DB::Exception("Detached part \"" + name + "\" not found" , ErrorCodes::BAD_DATA_PART_NAME);
}
ActionLock StorageMergeTree::getActionLock(StorageActionBlockType action_type)
{

View File

@ -124,7 +124,6 @@ private:
void clearColumnInPartition(const ASTPtr & partition, const Field & column_name, const Context & context);
void attachPartition(const ASTPtr & partition, bool part, const Context & context);
void replacePartitionFrom(const StoragePtr & source_table, const ASTPtr & partition, bool replace, const Context & context);
void validateDetachedPartName(const String & name) const;
friend class MergeTreeBlockOutputStream;
friend class MergeTreeData;

View File

@ -3554,6 +3554,7 @@ void StorageReplicatedMergeTree::attachPartition(const ASTPtr & partition, bool
Strings parts;
if (attach_part)
{
validateDetachedPartName(partition_id);
parts.push_back(partition_id);
}
else
@ -3566,6 +3567,7 @@ void StorageReplicatedMergeTree::attachPartition(const ASTPtr & partition, bool
{
String name = it.name();
MergeTreePartInfo part_info;
// TODO what if name contains "_tryN" suffix?
if (!MergeTreePartInfo::tryParsePartName(name, &part_info, format_version))
continue;
if (part_info.partition_id != partition_id)

View File

@ -2,6 +2,7 @@
#include <DataTypes/DataTypeString.h>
#include <DataTypes/DataTypesNumber.h>
#include <DataTypes/DataTypeNullable.h>
#include <DataStreams/OneBlockInputStream.h>
#include <ext/shared_ptr_helper.h>
#include <Storages/IStorage.h>
@ -31,13 +32,12 @@ protected:
setColumns(ColumnsDescription{{
{"database", std::make_shared<DataTypeString>()},
{"table", std::make_shared<DataTypeString>()},
{"partition_id", std::make_shared<DataTypeString>()},
{"partition_id", std::make_shared<DataTypeNullable>(std::make_shared<DataTypeString>())},
{"name", std::make_shared<DataTypeString>()},
{"reason", std::make_shared<DataTypeString>()},
{"min_block_number", std::make_shared<DataTypeInt64>()},
{"max_block_number", std::make_shared<DataTypeInt64>()},
{"level", std::make_shared<DataTypeUInt32>()},
{"directory_name", std::make_shared<DataTypeString>()}
{"reason", std::make_shared<DataTypeNullable>(std::make_shared<DataTypeString>())},
{"min_block_number", std::make_shared<DataTypeNullable>(std::make_shared<DataTypeInt64>())},
{"max_block_number", std::make_shared<DataTypeNullable>(std::make_shared<DataTypeInt64>())},
{"level", std::make_shared<DataTypeNullable>(std::make_shared<DataTypeUInt32>())}
}});
}
@ -63,13 +63,12 @@ protected:
int i = 0;
columns[i++]->insert(info.database);
columns[i++]->insert(info.table);
columns[i++]->insert(p.valid_name ? p.partition_id : "");
columns[i++]->insert(p.valid_name ? p.getPartName() : "");
columns[i++]->insert(p.prefix);
columns[i++]->insert(p.min_block);
columns[i++]->insert(p.max_block);
columns[i++]->insert(p.level);
columns[i++]->insert(p.fullDirName());
columns[i++]->insert(p.valid_name ? p.partition_id : Field());
columns[i++]->insert(p.dir_name);
columns[i++]->insert(p.valid_name ? p.prefix : Field());
columns[i++]->insert(p.valid_name ? p.min_block : Field());
columns[i++]->insert(p.valid_name ? p.max_block : Field());
columns[i++]->insert(p.valid_name ? p.level : Field());
}
}

View File

@ -13,5 +13,5 @@ OK
16 120
=== detached ===
0_5_5_0
delete_tmp_0_7_7
attaching_0_6_6
deleting_0_7_7_0
attaching_0_6_6_0

View File

@ -27,15 +27,18 @@ $CLICKHOUSE_CLIENT --query="INSERT INTO attach_partitions SELECT number FROM sys
$CLICKHOUSE_CLIENT --query="INSERT INTO attach_partitions SELECT number FROM system.numbers WHERE number % 2 = 1 LIMIT 8";
$CLICKHOUSE_CLIENT --query="ALTER TABLE attach_partitions DETACH PARTITION 0";
mkdir $ch_dir/data/$cur_db/attach_partitions/detached/0_5_5_0/ # broken part
cp -r $ch_dir/data/$cur_db/attach_partitions/detached/0_1_1_0/ $ch_dir/data/$cur_db/attach_partitions/detached/attaching_0_6_6_0/
cp -r $ch_dir/data/$cur_db/attach_partitions/detached/0_3_3_0/ $ch_dir/data/$cur_db/attach_partitions/detached/delete_tmp_0_7_7_0/
sudo -n mkdir $ch_dir/data/$cur_db/attach_partitions/detached/0_5_5_0/ 2>/dev/null || \
mkdir $ch_dir/data/$cur_db/attach_partitions/detached/0_5_5_0/ # broken part
sudo -n cp -r $ch_dir/data/$cur_db/attach_partitions/detached/0_1_1_0/ $ch_dir/data/$cur_db/attach_partitions/detached/attaching_0_6_6_0/ 2>/dev/null || \
cp -r $ch_dir/data/$cur_db/attach_partitions/detached/0_1_1_0/ $ch_dir/data/$cur_db/attach_partitions/detached/attaching_0_6_6_0/
sudo -n cp -r $ch_dir/data/$cur_db/attach_partitions/detached/0_3_3_0/ $ch_dir/data/$cur_db/attach_partitions/detached/deleting_0_7_7_0/ 2>/dev/null || \
cp -r $ch_dir/data/$cur_db/attach_partitions/detached/0_3_3_0/ $ch_dir/data/$cur_db/attach_partitions/detached/deleting_0_7_7_0/
$CLICKHOUSE_CLIENT --query="ALTER TABLE attach_partitions ATTACH PARTITION 0";
$CLICKHOUSE_CLIENT --query="SElECT name FROM system.parts WHERE table='attach_partitions' AND database='${cur_db}' ORDER BY name FORMAT TSV";
$CLICKHOUSE_CLIENT --query="SElECT count(), sum(n) FROM attach_partitions FORMAT TSV";
echo '=== detached ===';
$CLICKHOUSE_CLIENT --query="SELECT directory_name FROM system.detached_parts WHERE table='attach_partitions' AND database='${cur_db}' FORMAT TSV";
$CLICKHOUSE_CLIENT --query="SELECT name FROM system.detached_parts WHERE table='attach_partitions' AND database='${cur_db}' FORMAT TSV";
$CLICKHOUSE_CLIENT --query="DROP TABLE attach_partitions";
$CLICKHOUSE_CLIENT --query="SYSTEM START MERGES";

View File

@ -15,16 +15,19 @@ $CLICKHOUSE_CLIENT --query="INSERT INTO drop_detached SELECT number FROM system.
$CLICKHOUSE_CLIENT --query="INSERT INTO drop_detached SELECT number FROM system.numbers WHERE number % 2 = 1 LIMIT 8";
$CLICKHOUSE_CLIENT --query="ALTER TABLE drop_detached DETACH PARTITION 0";
mkdir $ch_dir/data/$cur_db/drop_detached/detached/attaching_0_6_6_0/
mkdir $ch_dir/data/$cur_db/drop_detached/detached/delete_tmp_0_7_7_0/
mkdir $ch_dir/data/$cur_db/drop_detached/detached/any_other_name/
sudo -n mkdir $ch_dir/data/$cur_db/drop_detached/detached/attaching_0_6_6_0/ 2>/dev/null || \
mkdir $ch_dir/data/$cur_db/drop_detached/detached/attaching_0_6_6_0/
sudo -n mkdir $ch_dir/data/$cur_db/drop_detached/detached/deleting_0_7_7_0/ 2>/dev/null || \
mkdir $ch_dir/data/$cur_db/drop_detached/detached/deleting_0_7_7_0/
sudo -n mkdir $ch_dir/data/$cur_db/drop_detached/detached/any_other_name/ 2>/dev/null || \
mkdir $ch_dir/data/$cur_db/drop_detached/detached/any_other_name/
$CLICKHOUSE_CLIENT --allow_drop_detached=1 --query="ALTER TABLE drop_detached DROP DETACHED PART '../1_2_2_0'" 2>&1 | grep "Invalid part name" > /dev/null && echo 'OK'
$CLICKHOUSE_CLIENT --allow_drop_detached=1 --query="ALTER TABLE drop_detached DROP DETACHED PART '0_1_1_0'"
$CLICKHOUSE_CLIENT --allow_drop_detached=1 --query="ALTER TABLE drop_detached DROP DETACHED PART 'attaching_0_6_6_0'"
$CLICKHOUSE_CLIENT --allow_drop_detached=1 --query="ALTER TABLE drop_detached DROP DETACHED PART 'delete_tmp_0_7_7_0'"
$CLICKHOUSE_CLIENT --allow_drop_detached=1 --query="ALTER TABLE drop_detached DROP DETACHED PART 'deleting_0_7_7_0'"
$CLICKHOUSE_CLIENT --allow_drop_detached=1 --query="ALTER TABLE drop_detached DROP DETACHED PART 'any_other_name'"
$CLICKHOUSE_CLIENT --query="SElECT directory_name FROM system.detached_parts WHERE table='drop_detached' AND database='${cur_db}' FORMAT TSV";
$CLICKHOUSE_CLIENT --query="SElECT name FROM system.detached_parts WHERE table='drop_detached' AND database='${cur_db}' FORMAT TSV";
$CLICKHOUSE_CLIENT --query="DROP TABLE drop_detached";
$CLICKHOUSE_CLIENT --query="SYSTEM START MERGES";