Skip to content

Commit 43d3d02

Browse files
kruallkruall
authored
Fix incorrect event response on put error (#26497)
1 parent c1a50c9 commit 43d3d02

File tree

5 files changed

+92
-23
lines changed

5 files changed

+92
-23
lines changed

‎ydb/core/keyvalue/keyvalue_state.cpp‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "keyvalue_storage_read_request.h"
44
#include "keyvalue_storage_request.h"
55
#include "keyvalue_trash_key_arbitrary.h"
6+
#include "keyvalue_utils.h"
67
#include <ydb/core/base/tablet.h>
78
#include <ydb/core/protos/counters_keyvalue.pb.h>
89
#include <ydb/core/protos/msgbus_kv.pb.h>
@@ -2819,21 +2820,6 @@ TPrepareResult TKeyValueState::PrepareCommands(NKikimrKeyValue::ExecuteTransacti
28192820
return {};
28202821
}
28212822

2822-
NKikimrKeyValue::Statuses::ReplyStatus ConvertStatus(NMsgBusProxy::EResponseStatus status) {
2823-
switch (status) {
2824-
case NMsgBusProxy::MSTATUS_ERROR:
2825-
return NKikimrKeyValue::Statuses::RSTATUS_ERROR;
2826-
case NMsgBusProxy::MSTATUS_TIMEOUT:
2827-
return NKikimrKeyValue::Statuses::RSTATUS_TIMEOUT;
2828-
case NMsgBusProxy::MSTATUS_REJECTED:
2829-
return NKikimrKeyValue::Statuses::RSTATUS_WRONG_LOCK_GENERATION;
2830-
case NMsgBusProxy::MSTATUS_INTERNALERROR:
2831-
return NKikimrKeyValue::Statuses::RSTATUS_INTERNAL_ERROR;
2832-
default:
2833-
return NKikimrKeyValue::Statuses::RSTATUS_INTERNAL_ERROR;
2834-
};
2835-
}
2836-
28372823
void TKeyValueState::ReplyError(const TActorContext &ctx, TString errorDescription,
28382824
NMsgBusProxy::EResponseStatus oldStatus, NKikimrKeyValue::Statuses::ReplyStatus newStatus,
28392825
THolder<TIntermediate> &intermediate, const TTabletStorageInfo *info) {

‎ydb/core/keyvalue/keyvalue_storage_request.cpp‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "keyvalue_flat_impl.h"
2+
#include "keyvalue_utils.h"
23

34
#include <ydb/core/base/tablet_pipe.h>
45
#include <ydb/core/base/hive.h>
@@ -531,13 +532,8 @@ class TKeyValueStorageRequest : public TActorBootstrapped<TKeyValueStorageReques
531532
NLog::EPriority logPriority = NLog::PRI_ERROR) {
532533
LOG_LOG_S(ctx, logPriority, NKikimrServices::KEYVALUE, errorDescription);
533534

534-
THolder<TEvKeyValue::TEvResponse> response(new TEvKeyValue::TEvResponse);
535-
if (IntermediateResults->HasCookie) {
536-
response->Record.SetCookie(IntermediateResults->Cookie);
537-
}
538-
response->Record.SetErrorReason(errorDescription);
539-
response->Record.SetStatus(status);
540-
ctx.Send(IntermediateResults->RespondTo, response.Release());
535+
std::unique_ptr<IEventBase> response = MakeErrorResponse(IntermediateResults.Get(), status, errorDescription);
536+
ctx.Send(IntermediateResults->RespondTo, std::move(response));
541537
IntermediateResults->IsReplied = true;
542538

543539
IntermediateResults->Stat.YellowStopChannels.reserve(YellowStopChannels.size());
@@ -758,7 +754,7 @@ class TKeyValueStorageRequest : public TActorBootstrapped<TKeyValueStorageReques
758754
new TEvBlobStorage::TEvPatch(
759755
originalGroupId, request.OriginalBlobId, request.PatchedBlobId, TLogoBlobID::MaxCookie,
760756
std::move(diffs), request.Diffs.size(), IntermediateResults->Deadline));
761-
757+
762758
const ui32 groupId = TabletInfo->GroupFor(request.PatchedBlobId.Channel(), request.PatchedBlobId.Generation());
763759
Y_VERIFY_S(groupId != Max<ui32>(), "Patch Blob# " << request.PatchedBlobId.ToString() << " is mapped to an invalid group (-1)!");
764760
ALOG_DEBUG(NKikimrServices::KEYVALUE, "KeyValue# " << TabletInfo->TabletID

‎ydb/core/keyvalue/keyvalue_utils.cpp‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
#include <ydb/core/keyvalue/keyvalue_utils.h>
2+
3+
namespace NKikimr {
4+
namespace NKeyValue {
5+
6+
NKikimrKeyValue::Statuses::ReplyStatus ConvertStatus(NMsgBusProxy::EResponseStatus status) {
7+
switch (status) {
8+
case NMsgBusProxy::MSTATUS_ERROR:
9+
return NKikimrKeyValue::Statuses::RSTATUS_ERROR;
10+
case NMsgBusProxy::MSTATUS_TIMEOUT:
11+
return NKikimrKeyValue::Statuses::RSTATUS_TIMEOUT;
12+
case NMsgBusProxy::MSTATUS_REJECTED:
13+
return NKikimrKeyValue::Statuses::RSTATUS_WRONG_LOCK_GENERATION;
14+
case NMsgBusProxy::MSTATUS_INTERNALERROR:
15+
return NKikimrKeyValue::Statuses::RSTATUS_INTERNAL_ERROR;
16+
default:
17+
return NKikimrKeyValue::Statuses::RSTATUS_INTERNAL_ERROR;
18+
};
19+
}
20+
21+
22+
template <typename TEvent>
23+
struct THasCookie : std::true_type {};
24+
25+
template <>
26+
struct THasCookie<TEvKeyValue::TEvGetStorageChannelStatusResponse> : std::false_type {};
27+
28+
29+
30+
template<typename TEvent>
31+
std::unique_ptr<IEventBase> MakeErrorResponseImpl(const TIntermediate *intermediateResults, NKikimrKeyValue::Statuses::ReplyStatus status, TString errorDescription) {
32+
std::unique_ptr<TEvent> response(new TEvent);
33+
if constexpr (THasCookie<TEvent>::value) {
34+
if (intermediateResults->HasCookie) {
35+
response->Record.set_cookie(intermediateResults->Cookie);
36+
}
37+
}
38+
response->Record.set_msg(errorDescription);
39+
response->Record.set_status(status);
40+
return response;
41+
}
42+
43+
44+
std::unique_ptr<IEventBase> MakeErrorResponse(const TIntermediate *intermediateResults, NMsgBusProxy::EResponseStatus status, TString errorDescription) {
45+
if (intermediateResults->EvType == TEvKeyValue::TEvRequest::EventType) {
46+
std::unique_ptr<TEvKeyValue::TEvResponse> response(new TEvKeyValue::TEvResponse);
47+
if (intermediateResults->HasCookie) {
48+
response->Record.SetCookie(intermediateResults->Cookie);
49+
}
50+
response->Record.SetErrorReason(errorDescription);
51+
response->Record.SetStatus(status);
52+
return response;
53+
}
54+
if (intermediateResults->EvType == TEvKeyValue::TEvRead::EventType) {
55+
return MakeErrorResponseImpl<TEvKeyValue::TEvReadResponse>(intermediateResults, ConvertStatus(status), errorDescription);
56+
}
57+
if (intermediateResults->EvType == TEvKeyValue::TEvReadRange::EventType) {
58+
return MakeErrorResponseImpl<TEvKeyValue::TEvReadRangeResponse>(intermediateResults, ConvertStatus(status), errorDescription);
59+
}
60+
if (intermediateResults->EvType == TEvKeyValue::TEvExecuteTransaction::EventType) {
61+
return MakeErrorResponseImpl<TEvKeyValue::TEvExecuteTransactionResponse>(intermediateResults, ConvertStatus(status), errorDescription);
62+
}
63+
if (intermediateResults->EvType == TEvKeyValue::TEvGetStorageChannelStatus::EventType) {
64+
return MakeErrorResponseImpl<TEvKeyValue::TEvGetStorageChannelStatusResponse>(intermediateResults, ConvertStatus(status), errorDescription);
65+
}
66+
67+
return nullptr;
68+
}
69+
70+
71+
} // NKeyValue
72+
} // NKikimr

‎ydb/core/keyvalue/keyvalue_utils.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#pragma once
2+
3+
#include <ydb/core/keyvalue/keyvalue_events.h>
4+
5+
namespace NKikimr {
6+
namespace NKeyValue {
7+
8+
NMsgBusProxy::EResponseStatus ConvertStatus(NKikimrKeyValue::Statuses::ReplyStatus status);
9+
10+
std::unique_ptr<IEventBase> MakeErrorResponse(const TIntermediate *intermediateResults, NMsgBusProxy::EResponseStatus status, TString errorDescription);
11+
12+
} // NKeyValue
13+
} // NKikimr

‎ydb/core/keyvalue/ya.make‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ SRCS(
3838
keyvalue_stored_state_data.cpp
3939
keyvalue_stored_state_data.h
4040
keyvalue_trash_key_arbitrary.h
41+
keyvalue_utils.cpp
42+
keyvalue_utils.h
4143
)
4244

4345
PEERDIR(

0 commit comments

Comments
 (0)