Generic lookup add timeouts #13239
Generic lookup add timeouts #13239
Conversation
yumkam
changed the title
|
⚪ |
|
⚪ |
|
⚪ |
|
⚪ |
|
⚪ |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
yumkam
force-pushed
the
generic-lookup-add-timeouts
branch
from
12883ed to
618be91
Compare
| @@ -28,6 +28,8 @@ namespace NYql::NDq { | |||
| using namespace NActors; | |||
|
|
|||
| namespace { | |||
| constexpr ui32 RequestRetriesLimit = 10; // TODO lookup parameters or PRAGMA? | |||
| constexpr TDuration RequestTimeout = TDuration::Minutes(3); // TODO lookup parameters or PRAGMA? | |||
There was a problem hiding this comment.
Это должна быть серверная настройка. В pragma мы такое пользователям не даем крутить. Давай лимит поставим 30 секунд, 3 минуты слишком много для ydb запроса. Какое сейчас максимальное время ответа?
There was a problem hiding this comment.
Это надо смотреть в реальных условиях; зависит от: загрузки базы, насколько она далеко по сети, сколько накопилось ключей и какого они размера (ну и первый запрос (per key number) ещё должен успеть откомпилироваться).
Локально я больше нескольких секунд не видел. Но, кмк, 30 секунд это слишком хрупко.
There was a problem hiding this comment.
В сумме 30 минут ретраить это слишком жестко, давай параметры подберем чтобы не больше 3-5 минут тогда было. Давай число ретраев тогда до 2-х уменьшим
| if (NConnector::GrpcStatusNeedsRetry(status) || status.GRpcStatusCode == grpc::DEADLINE_EXCEEDED) { | ||
| if (retriesRemaining) { | ||
| const auto retry = RequestRetriesLimit - retriesRemaining; | ||
| const auto delay = TDuration::MilliSeconds(1u << retry); // Exponential delay from 1ms to ~0.5s |
There was a problem hiding this comment.
У нас уже есть библиотека для ретраве. Используй готовую либу https://a.yandex-team.ru/arcadia/library/cpp/retry/retry_policy.h?rev=r15686703#L269
There was a problem hiding this comment.
Хмм... с одной стороны, замена велосипеда напрашивается. С другой, это задерживает фикс. Может, замержить as is и перенести замену ретраев в follow-up?
yumkam
force-pushed
the
generic-lookup-add-timeouts
branch
from
618be91 to
81552aa
Compare
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
yumkam
changed the title
(cherry picked from commit ea09754)
Changelog entry
...
Changelog category
Additional information
...