YQ-2068 use HashMap for lookup request and result #5594

zverevgeny · 2024-06-15T10:41:15Z

No description provided.

github-actions · 2024-06-15T10:44:32Z

⚪ 2024-06-15 10:44:31 UTC Pre-commit check for d770236 has started.
⚪ 2024-06-15 10:47:03 UTC Build linux-x86_64-release-clang14 is running...
🔴 2024-06-15 10:58:13 UTC Build failed. see the build logs.

github-actions · 2024-06-15T10:44:35Z

⚪ 2024-06-15 10:44:35 UTC Pre-commit check for d770236 has started.
⚪ 2024-06-15 10:47:14 UTC Build linux-x86_64-relwithdebinfo is running...
🔴 2024-06-15 11:00:10 UTC Build failed. see the build logs.
🔴 2024-06-15 11:00:11 UTC Tests run skipped.

github-actions · 2024-06-15T10:44:36Z

⚪ 2024-06-15 10:44:36 UTC Pre-commit check for d770236 has started.
⚪ 2024-06-15 10:47:09 UTC Build linux-x86_64-release-asan is running...
🔴 2024-06-15 11:00:00 UTC Build failed. see the build logs.
🔴 2024-06-15 11:00:02 UTC Tests run skipped.

uzhastik · 2024-06-15T11:17:00Z

ydb/library/yql/dq/actors/compute/dq_compute_actor_async_io.h

        ~TEvLookupRequest() {
            auto guard = Guard(*Alloc);
-            Keys = NKikimr::NMiniKQL::TUnboxedValueVector{};
+            Request = TUnboxedValueMap{0, {{}, false, nullptr}, {{}, false, nullptr}};


в чем ценность такой сложной конструкции? почему нельзя написать Request = {} ?

Упростил. Request = {} писать нельзя, т.к. не дефолтных конструкторов у TValueHasher и TValueEqual

ydb/library/yql/providers/generic/actors/ut/yql_generic_lookup_actor_ut.cpp

uzhastik · 2024-06-15T11:25:58Z

ydb/library/yql/providers/generic/actors/ut/yql_generic_lookup_actor_ut.cpp

-            auto& [k, v] = lookupResult[1];
-            UNIT_ASSERT_EQUAL(1, k.GetElement(0).Get<ui64>());
-            UNIT_ASSERT_EQUAL(101, k.GetElement(1).Get<ui64>());
+        } else UNIT_ASSERT(false);


давай избавимся от этих UNIT_ASSERT(false) и излишних ветвлений. мне такой стиль тяжело читать
вот так легче:

auto v = lookupResult->FindPtr(....); UNIT_ASSERT(v); NYql::NUdf::TUnboxedValue val = v->GetElement(0); UNIT_ASSERT(val.AsStringRef() == TStringBuf("a"));

переписал

uzhastik · 2024-06-15T11:26:42Z

ydb/library/yql/providers/generic/actors/ut/yql_generic_lookup_actor_ut.cpp

-            UNIT_ASSERT_EQUAL(0, k.GetElement(0).Get<ui64>());
-            UNIT_ASSERT_EQUAL(100, k.GetElement(1).Get<ui64>());
+        if (const auto* v = lookupResult->FindPtr(CreateStructValue(holderFactory, {0, 100}))) {
            NYql::NUdf::TUnboxedValue val = v.GetElement(0);


v - это же указатель вроде. почему тут v., а не v-> ?

переписал это место

ydb/library/yql/providers/generic/actors/yql_generic_lookup_actor.cpp

uzhastik · 2024-06-15T11:28:06Z

ydb/library/yql/providers/generic/actors/yql_generic_lookup_actor.cpp

        }
-        void AsyncLookup(const NKikimr::NMiniKQL::TUnboxedValueVector& keys) override {
+        void AsyncLookup(IDqAsyncLookupSource::TUnboxedValueMap&& request) override {
            auto guard = Guard(*Alloc);


guard выглядит бесполезным тут

uzhastik · 2024-06-15T11:29:35Z

ydb/library/yql/providers/yt/actors/ut/yql_yt_lookup_actor_ut.cpp

-        auto ev = new NDq::IDqAsyncLookupSource::TEvLookupRequest(Alloc, std::move(KeysToLookUp));
+        auto ev = new NDq::IDqAsyncLookupSource::TEvLookupRequest(Alloc, std::move(Request));
        TActivationContext::ActorSystem()->Send(new NActors::IEventHandle(LookupActor, SelfId(), ev));
        auto guard = Guard(*Alloc);


guard выглядит бесполезным

удалил

uzhastik · 2024-06-15T11:30:19Z

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

+        YQL_CLOG(DEBUG, ProviderYt) << "ActorId=" << SelfId() << " Got LookupRequest for " << request.size() << " keys";
        Y_ABORT_IF(InProgress);
-        Y_ABORT_IF(keys.size() > MaxKeysInRequest);
+        Y_ABORT_IF(request.size() > MaxKeysInRequest);


давай тут не падать, лучше исключение бросить

Это контракт, выраженный в коде. Трансформер сначала должен запросить у lookup актора максимально поддерживаемое кол-во ключей, а потом запрашивать не больше этого значения

github-actions · 2024-06-15T12:06:46Z

⚪ 2024-06-15 12:06:46 UTC Pre-commit check for 878968f has started.
⚪ 2024-06-15 12:09:21 UTC Build linux-x86_64-release-asan is running...
🟢 2024-06-15 12:24:19 UTC Build successful.
⚪ 2024-06-15 12:24:38 UTC Tests are running...
🔴 2024-06-15 14:12:12 UTC Some tests failed, follow the links below.

Test history | Test log

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
10952	10902	0	20	21	9

github-actions · 2024-06-15T12:06:53Z

⚪ 2024-06-15 12:06:53 UTC Pre-commit check for 878968f has started.
⚪ 2024-06-15 12:09:28 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-06-15 12:21:22 UTC Build successful.

github-actions · 2024-06-15T12:06:54Z

⚪ 2024-06-15 12:06:54 UTC Pre-commit check for 878968f has started.
⚪ 2024-06-15 12:09:28 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-06-15 12:21:39 UTC Build successful.
⚪ 2024-06-15 12:21:58 UTC Tests are running...
🔴 2024-06-15 14:08:10 UTC Some tests failed, follow the links below.

Test history | Test log

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
38988	34160	0	3	4818	7

github-actions · 2024-06-15T15:24:55Z

⚪ 2024-06-15 15:24:54 UTC Pre-commit check for 0647a5e has started.
⚪ 2024-06-15 15:27:33 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-06-15 15:39:31 UTC Build successful.

github-actions · 2024-06-15T15:25:10Z

⚪ 2024-06-15 15:25:10 UTC Pre-commit check for 0647a5e has started.
⚪ 2024-06-15 15:27:44 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-06-15 15:40:10 UTC Build successful.
⚪ 2024-06-15 15:40:28 UTC Tests are running...
🔴 2024-06-15 17:25:19 UTC Some tests failed, follow the links below.

Test history | Test log

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
38991	34161	0	2	4818	10

github-actions · 2024-06-15T15:25:12Z

⚪ 2024-06-15 15:25:12 UTC Pre-commit check for 0647a5e has started.
⚪ 2024-06-15 15:27:45 UTC Build linux-x86_64-release-asan is running...
🟢 2024-06-15 15:40:34 UTC Build successful.
⚪ 2024-06-15 15:40:50 UTC Tests are running...
🔴 2024-06-15 17:31:37 UTC Some tests failed, follow the links below.

Test history | Test log

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
10959	10911	0	16	20	12

zverevgeny requested review from a team as code owners June 15, 2024 10:41

uzhastik reviewed Jun 15, 2024

View reviewed changes

YQ-2068 use HashMap for lookup request and result

e32188e

zverevgeny force-pushed the YQ-2068_unordered_for_lookup_req_resp branch from 33f69f5 to e32188e Compare June 15, 2024 12:03

fix tests

ee10020

zverevgeny requested a review from uzhastik June 17, 2024 09:08

rvu1024 approved these changes Jun 17, 2024

View reviewed changes

uzhastik approved these changes Jun 18, 2024

View reviewed changes

zverevgeny merged commit 82687fa into ydb-platform:main Jun 18, 2024

This was referenced Jun 18, 2024

YQ-3342 fix script results writing #5682

Merged

WM passed resource pool settings to user request context #5734

Merged

YQ splitted fq/s3/test_s3.py tests #5798

Merged

This was referenced Jun 23, 2024

move time difference issue under database level #5859

Merged

metadata requests flags fix #5860

Merged

oidc-proxy new iam 2 #6059

Merged

YQ-2068 use HashMap for lookup request and result #5594

YQ-2068 use HashMap for lookup request and result #5594

Uh oh!

Conversation

zverevgeny commented Jun 15, 2024

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Jun 15, 2024 •

edited

Loading

github-actions bot commented Jun 15, 2024 •

edited

Loading

github-actions bot commented Jun 15, 2024 •

edited

Loading

github-actions bot commented Jun 15, 2024 •

edited

Loading

github-actions bot commented Jun 15, 2024 •

edited

Loading

github-actions bot commented Jun 15, 2024 •

edited

Loading

github-actions bot commented Jun 15, 2024 •

edited

Loading

github-actions bot commented Jun 15, 2024 •

edited

Loading

github-actions bot commented Jun 15, 2024 •

edited

Loading