-
Notifications
You must be signed in to change notification settings - Fork 606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support trailing generic query responses #1441
support trailing generic query responses #1441
Conversation
⚪
|
⚪
|
bb1736b
to
148e7bc
Compare
⚪
|
⚪
|
148e7bc
to
aeab9d4
Compare
⚪ |
⚪ |
aeab9d4
to
ab89e4b
Compare
⚪
|
⚪
|
@@ -77,6 +77,19 @@ Ydb::ResultSet* TKqpExecuterTxResult::GetYdb(google::protobuf::Arena* arena, TMa | |||
return ydbResult; | |||
} | |||
|
|||
Ydb::ResultSet* TKqpExecuterTxResult::GetTrailingYdb(google::protobuf::Arena* arena, TMaybe<ui64>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В чем смысл второго аргумента?
@@ -1532,6 +1538,23 @@ class TKqpSessionActor : public TActorBootstrapped<TKqpSessionActor> { | |||
|
|||
// Result for scan query is sent directly to target actor. | |||
Y_ABORT_UNLESS(response->GetArena()); | |||
if (QueryState->PreparedQuery && QueryState->IsStreamResult()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Эта секция очень похожа на следующую, где !QueryState->IsStreamResult().
Там есть какие-то принципиальные отличия?
auto ackEv = MakeHolder<NYql::NDq::TEvDqCompute::TEvChannelDataAck>(); | ||
ackEv->Record.SetSeqNo(computeData.Proto.GetSeqNo()); | ||
ackEv->Record.SetChannelId(channel.Id); | ||
ackEv->Record.SetFreeSpace(50_MB); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мы же проверили что канал уже FINISHED, какая ему разница на FreeSpace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да, просто нужно как-то заполнить финальный ак. не вижу тут проблемы какой-то.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну константы всегда вопросы вызывают, почему просто 0 не поставить? )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
короче, шаг влево - тесты начинают зависать. похоже что есть какие-то проверки на получателе, могу покопать отдельно.
this->Send(channelComputeActorId, ackEv.Release(), /* TODO: undelivery */ 0, /* cookie */ channel.Id); | ||
} | ||
|
||
void HandleStreamAck(TEvKqpExecuter::TEvStreamDataAck::TPtr& ev) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вот тут кстати не до конца понятно.
- Если используется механизм trailing result, то аков нам сюда не прилетит.
- Если не используется, то почему бы rpc актору не отправлять Ack'и напрямую в CA?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вообще идея норм, я поправлю, но пока я тут оставлю этот код на всякий.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
короче, шаг влево - тесты начинают зависать. похоже что есть какие-то проверки на получателе, могу покопать отдельно.
ab89e4b
to
fb174eb
Compare
fb174eb
to
e13b4f1
Compare
e13b4f1
to
d12fb13
Compare
⚪
|
d12fb13
to
66d80ab
Compare
⚪
|
⚪
|
⚪
|
Changelog entry
improve performance for single row responses in generic query
Changelog category
Additional information
...