use statistics aggregator for basic statistics #623
Conversation
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit cb802c1.
🔴 linux-x86_64-release-asan: some tests FAILED for commit cb802c1.
|
ydb/core/protos/statistics.proto
Outdated
| optional uint32 NodeId = 1; | ||
| optional bool HasStatistics = 2; | ||
| optional fixed64 SchemeShardId = 2; |
There was a problem hiding this comment.
а почему тут только один? и зачем это отличается от TEvSyncNode?
ydb/core/protos/statistics.proto
Outdated
| // nodes -> SA | ||
| message TEvSyncNode { | ||
| optional uint32 NodeId = 1; | ||
| repeated fixed64 SchemeShardIds = 2; |
There was a problem hiding this comment.
не хватает таймстемпа последней статистики - и тут и при регистрации - если она у ноды есть - ее можно не спешить ей слать
а если нет - надо послать ASAP
There was a problem hiding this comment.
добавил таймстемпы в протокол
| void TStatisticsAggregator::Handle(TEvTabletPipe::TEvServerConnected::TPtr &ev) { | ||
| auto pipeServerId = ev->Get()->ServerId; | ||
|
|
||
| SA_LOG_D("[" << TabletID() << "] EvServerConnected" |
There was a problem hiding this comment.
Все вызовы SA_LOG_D начинаются с ("[" << TabletID() << "]
как будто стоит сделать это частью макроса
There was a problem hiding this comment.
это лучше поправить отдельным коммитом
|
|
||
| if (FastCounter > 0) { | ||
| --FastCounter; | ||
| SendStatisticsToNode(nodeId, schemeShardId); |
There was a problem hiding this comment.
тут точно не пропущен return ?
There was a problem hiding this comment.
переписал логику
| for (const auto& [nodeId, _] : Nodes) { | ||
| nodeIds.push_back(nodeId); | ||
| } | ||
| auto device = std::random_device(); |
There was a problem hiding this comment.
а вот это точно не перебор - каждый раз ходить в аппаратный генератор?
мне кажется это дорого и нужно только в реальной криптографии.
В целом я бы рекомендовал получить обычный быстрый и качественный генератор - mersene twister например mt19937_64, инициировать его чем-то совсем дешевым типа текущего rdtsc однажды и дальше держать в классе aggregator экземпляр rng и когда надо только использовать из него значения например как тут в shuffle.
| nodeIds.push_back(nodeId); | ||
| } | ||
| auto device = std::random_device(); | ||
| auto rng = std::default_random_engine(device()); |
There was a problem hiding this comment.
и вот тут использовать уже ранее созданный генератор
| @@ -80,6 +104,27 @@ class TStatisticsAggregator : public TActor<TStatisticsAggregator>, public NTabl | |||
|
|
|||
| private: | |||
| TString Database; | |||
|
|
|||
| static constexpr size_t StatsOptimizeFirstNodesCount = 3; // optimize first nodes - fast propagation | |||
| static constexpr size_t StatsSizeLimitBytes = 40 << 20; // limit for stats size in one message | |||
There was a problem hiding this comment.
Я бы по умолчанию сделал 2 мегабайта и правило вида "пока не накопится 2 мегабайта но не менее 1 набора данных"
соображения такие: 2 мегабайта передаются по 10 гигабитной сети целых 16 миллисекунд, а 40 - уже 320. Лучше выстрелить в ноду серией запросов по 16 мс, акторная система скажет спасибо
There was a problem hiding this comment.
сделал 2 мегабайта
| @@ -4509,9 +4513,8 @@ void TSchemeShard::StateWork(STFUNC_SIG) { | |||
|
|
|||
| HFuncTraced(TEvPersQueue::TEvProposeTransactionAttachResult, Handle); | |||
|
|
|||
| HFuncTraced(TEvPrivate::TEvProcessStatistics, Handle); | |||
There was a problem hiding this comment.
Эти сообщения точно не могут отсылаться/получаться в stable-23-3 ?
There was a problem hiding this comment.
нет, в 23-3 этого функционала ещё не было, и туда я не мёрджил
alexd65536
force-pushed
the
base_stats_serverless
branch
from
01734c7 to
cb802c1
Compare
No description provided.