Skip to content

Commit 522cfdb

Browse files
committed
do not mark part export as failed in case of cancelled export
1 parent 460f2f4 commit 522cfdb

File tree

4 files changed

+36
-13
lines changed

4 files changed

+36
-13
lines changed

src/Storages/MergeTree/ExportPartTask.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ bool ExportPartTask::executeStep()
138138
storage.export_manifests.erase(manifest);
139139

140140
if (manifest.completion_callback)
141-
manifest.completion_callback(MergeTreePartExportManifest::CompletionCallbackResult::createFailure(e.message()));
141+
manifest.completion_callback(MergeTreePartExportManifest::CompletionCallbackResult::createFailure(e));
142142
return false;
143143
}
144144

@@ -255,7 +255,7 @@ bool ExportPartTask::executeStep()
255255
storage.export_manifests.erase(manifest);
256256

257257
if (manifest.completion_callback)
258-
manifest.completion_callback(MergeTreePartExportManifest::CompletionCallbackResult::createFailure(e.message()));
258+
manifest.completion_callback(MergeTreePartExportManifest::CompletionCallbackResult::createFailure(e));
259259

260260
throw;
261261
}

src/Storages/MergeTree/ExportPartitionTaskScheduler.cpp

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,21 @@
22
#include <Storages/StorageReplicatedMergeTree.h>
33
#include <Interpreters/Context.h>
44
#include <Interpreters/DatabaseCatalog.h>
5-
#include "Common/ZooKeeper/Types.h"
5+
#include <Common/Exception.h>
6+
#include <Common/ZooKeeper/Types.h>
67
#include "Storages/MergeTree/ExportPartitionUtils.h"
78
#include "Storages/MergeTree/MergeTreePartExportManifest.h"
89

910

1011
namespace DB
1112
{
1213

14+
namespace ErrorCodes
15+
{
16+
extern const int QUERY_WAS_CANCELLED;
17+
extern const int LOGICAL_ERROR;
18+
}
19+
1320
namespace
1421
{
1522
ContextPtr getContextCopyWithTaskSettings(const ContextPtr & context, const ExportReplicatedMergeTreePartitionManifest & manifest)
@@ -210,10 +217,22 @@ void ExportPartitionTaskScheduler::handlePartExportFailure(
210217
const std::string & part_name,
211218
const std::filesystem::path & export_path,
212219
const zkutil::ZooKeeperPtr & zk,
213-
const String & exception,
220+
const std::optional<Exception> & exception,
214221
size_t max_retries
215222
)
216223
{
224+
if (!exception)
225+
{
226+
throw Exception(ErrorCodes::LOGICAL_ERROR, "ExportPartition scheduler task: No exception provided for error handling. Sounds like a bug");
227+
}
228+
229+
/// Early exit if the query was cancelled - no need to increment error counts
230+
if (exception->code() == ErrorCodes::QUERY_WAS_CANCELLED)
231+
{
232+
LOG_INFO(storage.log, "ExportPartition scheduler task: Part {} export was cancelled, skipping error handling", part_name);
233+
return;
234+
}
235+
217236
Coordination::Stat locked_by_stat;
218237
std::string locked_by;
219238

@@ -274,15 +293,15 @@ void ExportPartitionTaskScheduler::handlePartExportFailure(
274293
num_exceptions = std::stoull(num_exceptions_string.c_str());
275294

276295
ops.emplace_back(zkutil::makeSetRequest(last_exception_path / "part", part_name, -1));
277-
ops.emplace_back(zkutil::makeSetRequest(last_exception_path / "exception", exception, -1));
296+
ops.emplace_back(zkutil::makeSetRequest(last_exception_path / "exception", exception->message(), -1));
278297
}
279298
else
280299
{
281300
ops.emplace_back(zkutil::makeCreateRequest(exceptions_per_replica_path, "", zkutil::CreateMode::Persistent));
282301
ops.emplace_back(zkutil::makeCreateRequest(count_path, "0", zkutil::CreateMode::Persistent));
283302
ops.emplace_back(zkutil::makeCreateRequest(last_exception_path, "", zkutil::CreateMode::Persistent));
284303
ops.emplace_back(zkutil::makeCreateRequest(last_exception_path / "part", part_name, zkutil::CreateMode::Persistent));
285-
ops.emplace_back(zkutil::makeCreateRequest(last_exception_path / "exception", exception, zkutil::CreateMode::Persistent));
304+
ops.emplace_back(zkutil::makeCreateRequest(last_exception_path / "exception", exception->message(), zkutil::CreateMode::Persistent));
286305
}
287306

288307
num_exceptions++;

src/Storages/MergeTree/ExportPartitionTaskScheduler.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace DB
77
{
88

9+
class Exception;
910
class StorageReplicatedMergeTree;
1011

1112
struct ExportReplicatedMergeTreePartitionManifest;
@@ -44,7 +45,7 @@ class ExportPartitionTaskScheduler
4445
const std::string & part_name,
4546
const std::filesystem::path & export_path,
4647
const zkutil::ZooKeeperPtr & zk,
47-
const String & exception,
48+
const std::optional<Exception> & exception,
4849
size_t max_retries);
4950

5051
bool tryToMovePartToProcessed(

src/Storages/MergeTree/MergeTreePartExportManifest.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33
#include <Interpreters/StorageID.h>
44
#include <Storages/MergeTree/IMergeTreeDataPart.h>
55
#include <QueryPipeline/QueryPipeline.h>
6+
#include <optional>
67

78
namespace DB
89
{
910

11+
class Exception;
12+
1013
class ExportPartTask;
1114

1215
struct MergeTreePartExportManifest
@@ -23,23 +26,23 @@ struct MergeTreePartExportManifest
2326
struct CompletionCallbackResult
2427
{
2528
private:
26-
CompletionCallbackResult(bool success_, const String & relative_path_in_destination_storage_, const String & exception_)
27-
: success(success_), relative_path_in_destination_storage(relative_path_in_destination_storage_), exception(exception_) {}
29+
CompletionCallbackResult(bool success_, const String & relative_path_in_destination_storage_, std::optional<Exception> exception_)
30+
: success(success_), relative_path_in_destination_storage(relative_path_in_destination_storage_), exception(std::move(exception_)) {}
2831
public:
2932

3033
static CompletionCallbackResult createSuccess(const String & relative_path_in_destination_storage_)
3134
{
32-
return CompletionCallbackResult(true, relative_path_in_destination_storage_, "");
35+
return CompletionCallbackResult(true, relative_path_in_destination_storage_, std::nullopt);
3336
}
3437

35-
static CompletionCallbackResult createFailure(const String & exception_)
38+
static CompletionCallbackResult createFailure(Exception exception_)
3639
{
37-
return CompletionCallbackResult(false, "", exception_);
40+
return CompletionCallbackResult(false, "", std::move(exception_));
3841
}
3942

4043
bool success = false;
4144
String relative_path_in_destination_storage;
42-
String exception;
45+
std::optional<Exception> exception;
4346
};
4447

4548
MergeTreePartExportManifest(

0 commit comments

Comments
 (0)