From e57069c1282620f798160c1a42a2f96313a4ef79 Mon Sep 17 00:00:00 2001 From: Filitov Mikhail Date: Thu, 5 Sep 2024 17:01:36 +0300 Subject: [PATCH 1/5] Better spilling errors --- .../yql/dq/actors/spilling/channel_storage_actor.cpp | 2 +- .../yql/dq/actors/spilling/compute_storage_actor.cpp | 4 ++-- ydb/library/yql/dq/actors/spilling/spilling_file.cpp | 10 ++++++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp b/ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp index c3ce1c18bbc7..2b79f115541e 100644 --- a/ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp +++ b/ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp @@ -82,7 +82,7 @@ class TDqChannelStorageActor : public IDqChannelStorageActor, if (!ErrorCallback_) Y_ABORT("Error: %s", error.c_str()); LOG_E("Error: " << error); - ErrorCallback_(error); + ErrorCallback_(TStringBuilder() << "[Spilling]" << error); SendInternal(SpillingActorId_, new TEvents::TEvPoison); PassAway(); } diff --git a/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp b/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp index bd12bb23b80b..1b481a7eaf32 100644 --- a/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp +++ b/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp @@ -76,7 +76,7 @@ class TDqComputeStorageActor : public NActors::TActorBootstrappedGet(); - FailWithError(TStringBuilder() << "[TEvError] " << msg.Message); + FailWithError(TStringBuilder() << "[Spilling][TEvError] " << msg.Message); } bool HandleDelete(TKey blobId, ui64 size) { diff --git a/ydb/library/yql/dq/actors/spilling/spilling_file.cpp b/ydb/library/yql/dq/actors/spilling/spilling_file.cpp index 304433a26472..49b8fba0781f 100644 --- a/ydb/library/yql/dq/actors/spilling/spilling_file.cpp +++ b/ydb/library/yql/dq/actors/spilling/spilling_file.cpp @@ -1,6 +1,7 @@ #include "spilling.h" #include "spilling_file.h" +#include #include #include @@ -406,7 +407,10 @@ class TDqLocalFileSpillingService : public TActorBootstrappedSender << ", blobId: " << msg.BlobId << ", bytes: " << msg.Blob.size()); - Send(ev->Sender, new TEvDqSpilling::TEvError("File size limit exceeded")); + const auto usedMb = (fd.TotalSize + msg.Blob.size()) / 1024 / 1024; + const auto limitMb = Config_.MaxFileSize / 1024 / 1024; + + Send(ev->Sender, new TEvDqSpilling::TEvError(std::format("File size limit exceeded: {}/{}Mb", usedMb, limitMb))); Counters_->SpillingTooBigFileErrors->Inc(); return; @@ -416,7 +420,9 @@ class TDqLocalFileSpillingService : public TActorBootstrappedSender << ", blobId: " << msg.BlobId << ", bytes: " << msg.Blob.size()); - Send(ev->Sender, new TEvDqSpilling::TEvError("Total size limit exceeded")); + const auto usedMb = (TotalSize_ + msg.Blob.size()) / 1024 / 1024; + const auto limitMb = Config_.MaxTotalSize / 1024 / 1024; + Send(ev->Sender, new TEvDqSpilling::TEvError(std::format("Total size limit exceeded: {}/{}Mb", usedMb, limitMb))); Counters_->SpillingNoSpaceErrors->Inc(); return; From c2c4fd051827e1eb22edb4c81abe1d03e99d453d Mon Sep 17 00:00:00 2001 From: Filitov Mikhail Date: Thu, 5 Sep 2024 17:03:24 +0300 Subject: [PATCH 2/5] fixup --- ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp b/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp index 1b481a7eaf32..1f10bc439e9c 100644 --- a/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp +++ b/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp @@ -224,7 +224,7 @@ class TDqComputeStorageActor : public NActors::TActorBootstrappedGet(); - FailWithError(TStringBuilder() << "[Spilling][TEvError] " << msg.Message); + FailWithError(TStringBuilder() << "[TEvError] " << msg.Message); } bool HandleDelete(TKey blobId, ui64 size) { From 1d0d5c341057a01730597cf53775e8e8826d12f0 Mon Sep 17 00:00:00 2001 From: Filitov Mikhail Date: Thu, 5 Sep 2024 17:05:20 +0300 Subject: [PATCH 3/5] fixup --- ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp | 2 +- ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp b/ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp index 2b79f115541e..d050d54ac50d 100644 --- a/ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp +++ b/ydb/library/yql/dq/actors/spilling/channel_storage_actor.cpp @@ -82,7 +82,7 @@ class TDqChannelStorageActor : public IDqChannelStorageActor, if (!ErrorCallback_) Y_ABORT("Error: %s", error.c_str()); LOG_E("Error: " << error); - ErrorCallback_(TStringBuilder() << "[Spilling]" << error); + ErrorCallback_(TStringBuilder() << "[Channel spilling]" << error); SendInternal(SpillingActorId_, new TEvents::TEvPoison); PassAway(); } diff --git a/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp b/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp index 1f10bc439e9c..089b16cc6f91 100644 --- a/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp +++ b/ydb/library/yql/dq/actors/spilling/compute_storage_actor.cpp @@ -76,7 +76,7 @@ class TDqComputeStorageActor : public NActors::TActorBootstrapped Date: Thu, 5 Sep 2024 18:29:07 +0300 Subject: [PATCH 4/5] added test --- ydb/core/kqp/ut/spilling/kqp_scan_spilling_ut.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ydb/core/kqp/ut/spilling/kqp_scan_spilling_ut.cpp b/ydb/core/kqp/ut/spilling/kqp_scan_spilling_ut.cpp index ab5ff3828728..0994766005e0 100644 --- a/ydb/core/kqp/ut/spilling/kqp_scan_spilling_ut.cpp +++ b/ydb/core/kqp/ut/spilling/kqp_scan_spilling_ut.cpp @@ -135,7 +135,12 @@ Y_UNIT_TEST(HandleErrorsCorrectly) { Cerr << planres.GetStats()->GetAst() << Endl; auto result = db.ExecuteQuery(SimpleGraceJoinWithSpillingQuery, NYdb::NQuery::TTxControl::BeginTx().CommitTx(), NYdb::NQuery::TExecuteQuerySettings()).ExtractValueSync(); - UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::INTERNAL_ERROR, result.GetIssues().ToString()); + const auto errorMsg = result.GetIssues().ToString(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::INTERNAL_ERROR, errorMsg); + + const auto spillingPrefix = "[Compute spilling]"; + const auto pos = errorMsg.find(spillingPrefix); + UNIT_ASSERT_VALUES_UNEQUAL_C(pos, std::string::npos, "Spilling prefix not found in error message"); } Y_UNIT_TEST(SelfJoinQueryService) { From a036defbfa4cbeb1a8f916a84022ec73bb980bba Mon Sep 17 00:00:00 2001 From: Filitov Mikhail Date: Thu, 5 Sep 2024 22:07:38 +0300 Subject: [PATCH 5/5] fixed tests --- ydb/library/yql/dq/actors/spilling/spilling_file_ut.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ydb/library/yql/dq/actors/spilling/spilling_file_ut.cpp b/ydb/library/yql/dq/actors/spilling/spilling_file_ut.cpp index 6fd1be5affd5..cdf7a31cd8f8 100644 --- a/ydb/library/yql/dq/actors/spilling/spilling_file_ut.cpp +++ b/ydb/library/yql/dq/actors/spilling/spilling_file_ut.cpp @@ -270,7 +270,7 @@ Y_UNIT_TEST_SUITE(DqSpillingFileTests) { runtime.Send(new IEventHandle(spillingActor, tester, ev)); auto resp = runtime.GrabEdgeEvent(tester); - UNIT_ASSERT_STRINGS_EQUAL("Total size limit exceeded", resp->Get()->Message); + UNIT_ASSERT_STRINGS_EQUAL("Total size limit exceeded: 0/0Mb", resp->Get()->Message); } } @@ -297,7 +297,7 @@ Y_UNIT_TEST_SUITE(DqSpillingFileTests) { runtime.Send(new IEventHandle(spillingActor, tester, ev)); auto resp = runtime.GrabEdgeEvent(tester); - UNIT_ASSERT_STRINGS_EQUAL("File size limit exceeded", resp->Get()->Message); + UNIT_ASSERT_STRINGS_EQUAL("File size limit exceeded: 0/0Mb", resp->Get()->Message); } }