Skip to content
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

Merged

Conversation

gridnevvvit
Copy link
Member

Changelog entry

improve performance for single row responses in generic query

Changelog category

  • Improvement

Additional information

...

Copy link

github-actions bot commented Jan 30, 2024

2024-01-30 20:46:59 UTC Pre-commit check for ded3818 has started.
2024-01-30 20:47:01 UTC Build linux-x86_64-release-asan is running...
🟢 2024-01-30 21:23:06 UTC Build successful.
2024-01-30 21:23:18 UTC Tests are running...
🔴 2024-01-30 23:03:45 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
16022 15728 0 52 53 189

Copy link

github-actions bot commented Jan 30, 2024

2024-01-30 20:47:00 UTC Pre-commit check for ded3818 has started.
2024-01-30 20:47:03 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-01-30 21:22:45 UTC Build successful.
2024-01-30 21:23:01 UTC Tests are running...
🔴 2024-01-30 22:52:49 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
60221 50730 0 44 9263 184

@gridnevvvit gridnevvvit force-pushed the trailing-generic-query-responses branch from bb1736b to 148e7bc Compare January 31, 2024 16:55
Copy link

github-actions bot commented Jan 31, 2024

2024-01-31 18:01:47 UTC Pre-commit check for 95fc2b4 has started.
2024-01-31 18:01:48 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-01-31 18:38:38 UTC Build successful.
2024-01-31 18:38:49 UTC Tests are running...
🔴 2024-01-31 20:28:45 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
60412 49815 0 605 9397 595

Copy link

github-actions bot commented Jan 31, 2024

2024-01-31 18:02:32 UTC Pre-commit check for 95fc2b4 has started.
2024-01-31 18:02:34 UTC Build linux-x86_64-release-asan is running...
🟢 2024-01-31 18:38:38 UTC Build successful.
2024-01-31 18:38:49 UTC Tests are running...
🔴 2024-01-31 20:35:22 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
16086 14711 0 621 193 561

@gridnevvvit gridnevvvit force-pushed the trailing-generic-query-responses branch from 148e7bc to aeab9d4 Compare January 31, 2024 22:27
Copy link

github-actions bot commented Jan 31, 2024

2024-01-31 22:30:45 UTC Pre-commit check for 0e90a02 has started.
2024-01-31 22:30:48 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-01-31 23:09:38 UTC Build successful.
2024-01-31 23:09:51 UTC Tests are running...
🔴 2024-01-31 23:15:22 UTC Test run completed, no test results found for commit aeab9d4. Please check build logs.
2024-01-31 23:15:26 UTC Check cancelled

Copy link

github-actions bot commented Jan 31, 2024

2024-01-31 22:30:53 UTC Pre-commit check for 0e90a02 has started.
2024-01-31 22:30:55 UTC Build linux-x86_64-release-asan is running...
🟢 2024-01-31 23:07:37 UTC Build successful.
2024-01-31 23:07:54 UTC Tests are running...
🔴 2024-01-31 23:15:34 UTC Test run completed, no test results found for commit aeab9d4. Please check build logs.
2024-01-31 23:15:37 UTC Check cancelled

@gridnevvvit gridnevvvit force-pushed the trailing-generic-query-responses branch from aeab9d4 to ab89e4b Compare January 31, 2024 23:14
Copy link

github-actions bot commented Jan 31, 2024

2024-01-31 23:16:56 UTC Pre-commit check for 9d1fa49 has started.
2024-01-31 23:16:58 UTC Build linux-x86_64-release-asan is running...
🟢 2024-01-31 23:19:11 UTC Build successful.
2024-01-31 23:19:24 UTC Tests are running...
🔴 2024-02-01 00:55:18 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
16040 15930 0 16 50 44

Copy link

github-actions bot commented Jan 31, 2024

2024-01-31 23:19:11 UTC Pre-commit check for 9d1fa49 has started.
2024-01-31 23:19:13 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-01-31 23:21:37 UTC Build successful.
2024-01-31 23:21:51 UTC Tests are running...
🟢 2024-02-01 00:49:23 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
60356 51043 0 0 9270 43

ydb/core/grpc_services/query/rpc_execute_query.cpp Outdated Show resolved Hide resolved
ydb/core/kqp/common/events/query.h Show resolved Hide resolved
@@ -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>) {
Copy link
Collaborator

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()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Эта секция очень похожа на следующую, где !QueryState->IsStreamResult().
Там есть какие-то принципиальные отличия?

ydb/core/kqp/executer_actor/kqp_executer_impl.h Outdated Show resolved Hide resolved
auto ackEv = MakeHolder<NYql::NDq::TEvDqCompute::TEvChannelDataAck>();
ackEv->Record.SetSeqNo(computeData.Proto.GetSeqNo());
ackEv->Record.SetChannelId(channel.Id);
ackEv->Record.SetFreeSpace(50_MB);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мы же проверили что канал уже FINISHED, какая ему разница на FreeSpace?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

да, просто нужно как-то заполнить финальный ак. не вижу тут проблемы какой-то.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну константы всегда вопросы вызывают, почему просто 0 не поставить? )

Copy link
Member Author

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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот тут кстати не до конца понятно.

  1. Если используется механизм trailing result, то аков нам сюда не прилетит.
  2. Если не используется, то почему бы rpc актору не отправлять Ack'и напрямую в CA?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вообще идея норм, я поправлю, но пока я тут оставлю этот код на всякий.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

короче, шаг влево - тесты начинают зависать. похоже что есть какие-то проверки на получателе, могу покопать отдельно.

ydb/core/grpc_services/query/rpc_execute_query.cpp Outdated Show resolved Hide resolved
@gridnevvvit gridnevvvit force-pushed the trailing-generic-query-responses branch from ab89e4b to fb174eb Compare February 2, 2024 16:13
Copy link

github-actions bot commented Feb 2, 2024

2024-02-02 16:16:26 UTC Pre-commit check for 2baeee6 has started.
2024-02-02 16:16:28 UTC Build linux-x86_64-relwithdebinfo is running...
2024-02-02 16:21:02 UTC Check cancelled

Copy link

github-actions bot commented Feb 2, 2024

2024-02-02 16:18:43 UTC Pre-commit check for 2baeee6 has started.
2024-02-02 16:18:45 UTC Build linux-x86_64-release-asan is running...
2024-02-02 16:21:03 UTC Check cancelled

@gridnevvvit gridnevvvit force-pushed the trailing-generic-query-responses branch from fb174eb to e13b4f1 Compare February 2, 2024 16:20
Copy link

github-actions bot commented Feb 2, 2024

2024-02-02 16:24:38 UTC Pre-commit check for 7208485 has started.
2024-02-02 16:24:41 UTC Build linux-x86_64-relwithdebinfo is running...
2024-02-02 16:51:31 UTC Check cancelled

Copy link

github-actions bot commented Feb 2, 2024

2024-02-02 16:26:59 UTC Pre-commit check for 7208485 has started.
2024-02-02 16:27:01 UTC Build linux-x86_64-release-asan is running...
2024-02-02 16:51:32 UTC Check cancelled

@gridnevvvit gridnevvvit force-pushed the trailing-generic-query-responses branch from e13b4f1 to d12fb13 Compare February 2, 2024 16:51
Copy link

github-actions bot commented Feb 2, 2024

2024-02-03 10:39:14 UTC Pre-commit check for 12af35a has started.
2024-02-03 10:39:17 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-02-03 10:40:31 UTC Build successful.
2024-02-03 10:40:49 UTC Tests are running...
🔴 2024-02-03 12:01:45 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
58860 49567 0 3 9263 27

@gridnevvvit gridnevvvit force-pushed the trailing-generic-query-responses branch from d12fb13 to 66d80ab Compare February 2, 2024 16:53
Copy link

github-actions bot commented Feb 2, 2024

2024-02-03 14:44:11 UTC Pre-commit check for 50a622b has started.
2024-02-03 14:44:14 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-02-03 14:45:26 UTC Build successful.
2024-02-03 14:45:42 UTC Tests are running...
🔴 2024-02-03 14:53:13 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
58760 49458 0 6 9268 28

Copy link

github-actions bot commented Feb 2, 2024

2024-02-02 16:55:11 UTC Pre-commit check for 50a622b has started.
2024-02-02 16:55:14 UTC Build linux-x86_64-release-asan is running...
🟢 2024-02-02 17:10:14 UTC Build successful.
2024-02-02 17:10:27 UTC Tests are running...
🔴 2024-02-02 18:46:22 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14533 14399 0 41 61 32

Copy link

github-actions bot commented Feb 3, 2024

2024-02-03 10:39:25 UTC Pre-commit check for 12af35a has started.
2024-02-03 10:39:27 UTC Build linux-x86_64-release-asan is running...
🟢 2024-02-03 10:40:37 UTC Build successful.
2024-02-03 10:40:50 UTC Tests are running...
🔴 2024-02-03 12:16:48 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14530 14395 0 31 53 51

@gridnevvvit gridnevvvit merged commit 5594dac into ydb-platform:main Feb 3, 2024
2 of 4 checks passed
@gridnevvvit gridnevvvit deleted the trailing-generic-query-responses branch February 3, 2024 16:34
@starlinskiy starlinskiy mentioned this pull request Feb 12, 2024
@vitstn vitstn mentioned this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants