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.

Sign up for GitHub

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

Jump to bottom

YQ-2068 YT emu lookup source actor #4869

Merged
merged 3 commits into from
May 28, 2024
Merged

YQ-2068 YT emu lookup source actor #4869

merged 3 commits into from
May 28, 2024

Conversation

zverevgeny
Copy link
Collaborator

@zverevgeny zverevgeny commented May 27, 2024 •
edited
Loading

Changelog category

  • Not for changelog (changelog entry is not required)

@zverevgeny zverevgeny requested a review from a team as a code owner May 27, 2024 05:19
@github-actions GitHub Actions
Copy link

github-actions bot commented May 27, 2024 •
edited
Loading

⚪ 2024-05-27 05:23:11 UTC Pre-commit check for 171f777 has started.
⚪ 2024-05-27 05:25:19 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-27 05:26:54 UTC Build successful.
⚪ 2024-05-27 05:27:08 UTC Tests are running...
🔴 2024-05-27 05:44:18 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
749 733 0 4 12 0

@github-actions GitHub Actions
Copy link

github-actions bot commented May 27, 2024 •
edited
Loading

⚪ 2024-05-27 05:23:12 UTC Pre-commit check for 171f777 has started.
⚪ 2024-05-27 05:25:18 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-27 05:28:04 UTC Build successful.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 27, 2024 •
edited
Loading

⚪ 2024-05-27 05:23:14 UTC Pre-commit check for 171f777 has started.
⚪ 2024-05-27 05:25:23 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-27 05:27:27 UTC Build successful.
⚪ 2024-05-27 05:27:38 UTC Tests are running...
🟢 2024-05-27 06:16:43 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
58613 45942 0 0 12671 0

rvu1024
rvu1024 reviewed May 27, 2024
View reviewed changes
ydb/library/yql/providers/yt/actors/yql_yt_lookup_actor.cpp Outdated
return MaxKeysInRequest;
}
void AsyncLookup(const NKikimr::NMiniKQL::TUnboxedValueVector& keys) override {
YQL_CLOG(INFO, ProviderGeneric) << "ActorId=" << SelfId() << " Got LookupRequest for " << keys.size() << " keys";
Copy link
Member

Choose a reason for hiding this comment

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

Почему ProviderGeneric? И кажется INFO слишком вербозный для ээтого события

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

ydb/library/yql/providers/yt/actors/yql_yt_lookup_actor.cpp Outdated
auto guard = Guard(*Alloc);
KeyTypeHelper = TKeyTypeHelper{};
Data = TTableData(0, KeyTypeHelper.GetValueHash(), KeyTypeHelper.GetValueEqual());
Data.clear();
Copy link
Member

Choose a reason for hiding this comment

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

выглядит лишним вместе с предыдущей строкой

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

ydb/library/yql/providers/yt/actors/yql_yt_lookup_actor.cpp Outdated
, TypeEnv(typeEnv)
, MaxKeysInRequest(maxKeysInRequest)
, KeyTypeHelper(keyType)
, Data(1000,
Copy link
Member

Choose a reason for hiding this comment

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

Это reserve capacity? Зачем такой большой с учетом использования только для тестов?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

ydb/library/yql/providers/yt/actors/yql_yt_lookup_actor.cpp Outdated


void Bootstrap() {
YQL_CLOG(INFO, ProviderGeneric) << "New Yt proivider lookup source actor(ActorId=" << SelfId() << ") for"
Copy link
Member

Choose a reason for hiding this comment

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

Почему ProviderGeneric?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

ydb/library/yql/providers/yt/actors/yql_yt_lookup_actor.cpp Outdated
Comment on lines 170 to 171
Y_ABORT_UNLESS(lookupResult.back().first.IsBoxed());
Y_ABORT_UNLESS(!lookupResult.back().second || lookupResult.back().second.IsBoxed());
Copy link
Member

Choose a reason for hiding this comment

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

Откуда эти ограничения?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

ydb/library/yql/providers/yt/actors/yql_yt_lookup_actor.h

namespace NYql::NDq {

std::pair<NYql::NDq::IDqAsyncLookupSource*, NActors::IActor*> CreateYtLookupActor(
Copy link
Member

Choose a reason for hiding this comment

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

Где это подключается кроме ut?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

будет в dq

@github-actions GitHub Actions
Copy link

github-actions bot commented May 27, 2024 •
edited
Loading

⚪ 2024-05-27 16:35:30 UTC Pre-commit check for f219f0a has started.
⚪ 2024-05-27 16:37:35 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-27 16:38:56 UTC Build successful.
⚪ 2024-05-27 16:39:08 UTC Tests are running...
🔴 2024-05-27 16:55:24 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
749 735 0 2 12 0

@github-actions GitHub Actions
Copy link

github-actions bot commented May 27, 2024 •
edited
Loading

⚪ 2024-05-27 16:37:55 UTC Pre-commit check for f219f0a has started.
⚪ 2024-05-27 16:40:08 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-27 16:41:55 UTC Build successful.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 27, 2024 •
edited
Loading

⚪ 2024-05-27 16:38:13 UTC Pre-commit check for f219f0a has started.
⚪ 2024-05-27 16:40:26 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-27 16:42:41 UTC Build successful.
⚪ 2024-05-27 16:42:52 UTC Tests are running...
🟢 2024-05-27 17:33:14 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
58614 45943 0 0 12671 0

@zverevgeny zverevgeny requested a review from rvu1024 May 27, 2024 16:39
@github-actions GitHub Actions
Copy link

github-actions bot commented May 28, 2024 •
edited
Loading

⚪ 2024-05-28 04:00:02 UTC Pre-commit check for d7829eb has started.
⚪ 2024-05-28 04:02:12 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-28 04:03:49 UTC Build successful.
⚪ 2024-05-28 04:04:04 UTC Tests are running...
🔴 2024-05-28 04:21:23 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
750 734 0 4 12 0

@github-actions GitHub Actions
Copy link

github-actions bot commented May 28, 2024 •
edited
Loading

⚪ 2024-05-28 04:00:07 UTC Pre-commit check for d7829eb has started.
⚪ 2024-05-28 04:02:17 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-28 04:03:52 UTC Build successful.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 28, 2024 •
edited
Loading

⚪ 2024-05-28 04:02:29 UTC Pre-commit check for d7829eb has started.
⚪ 2024-05-28 04:04:35 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-28 04:06:08 UTC Build successful.
⚪ 2024-05-28 04:06:23 UTC Tests are running...
🟢 2024-05-28 04:55:22 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
58632 45959 0 0 12673 0

@zverevgeny zverevgeny merged commit 7389dc6 into ydb-platform:main May 28, 2024
10 of 12 checks passed
@niksaveliev niksaveliev mentioned this pull request May 29, 2024
Merged
This was referenced May 29, 2024
Merged
Merged
@niksaveliev niksaveliev mentioned this pull request May 30, 2024
Merged
@StekPerepolnen StekPerepolnen mentioned this pull request May 31, 2024
Merged
This was referenced Jun 6, 2024
Closed
Closed
Merged
Merged
@serbel324 serbel324 mentioned this pull request Jun 7, 2024
Merged
@niksaveliev niksaveliev mentioned this pull request Jun 10, 2024
Merged
This was referenced Jun 11, 2024
Merged
Merged
Merged
Merged
@niksaveliev niksaveliev mentioned this pull request Jun 17, 2024
Closed
This was referenced Jun 19, 2024
Merged
Merged
Merged
@StekPerepolnen StekPerepolnen mentioned this pull request Jun 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvu1024 rvu1024 rvu1024 approved these changes

Assignees
No one assigned
Labels
new-feature not-for-changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zverevgeny @rvu1024