-
Notifications
You must be signed in to change notification settings - Fork 38
[NBS] Pull scheme for non replicated disks #4792
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
base: main
Are you sure you want to change the base?
[NBS] Pull scheme for non replicated disks #4792
Conversation
|
Hi! Thank you for contributing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a title, a description and link an issue to this pr
komarevtsev-d
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM; Смотрел на эти изменения в другом ПРе
|
|
||
| auto* msg = ev->Get(); | ||
|
|
||
| if (msg->SeqNo < StatisticSeqNo) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В каком случае могут перепутаться запросы?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сейчас это больше перестраховка.
Если ответа на запрос статистики не поступило за 15 секунд, а при перепосылке запроса поступил какой-то ответ, то мы хотим быть уверены, что это новый, а не какой-то из прошлого
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А точно нужна эта перестраховка? Сообщения локальные, в одном процессе.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну страхуемся от багов короче, скорее так. Можно и убрать наверно.
| void Bootstrap(const NActors::TActorContext& ctx); | ||
|
|
||
| private: | ||
| void SendStatistics(const NActors::TActorContext& ctx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ReplyAndDie
| release_devices_actor.cpp | ||
| shadow_disk_actor.cpp | ||
| volume_as_partition_actor.cpp | ||
| partition_statistics_collector_actor.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
partition_statistics_collector_actor.cpp уже есть в ya.make
| std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>( | ||
| MakeIntrusive<TCallContext>(), | ||
| std::move(diskCounters), | ||
| PartConfig->GetName(), | ||
| networkBytes, | ||
| cpuUsage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
М.б. заиспользовать TPartNonreplCountersData в TDiskRegistryBasedPartitionCounters:
struct TDiskRegistryBasedPartitionCounters
{
TString DiskId;
TPartNonreplCountersData CountersData;
TDiskRegistryBasedPartitionCounters(
TString diskId,
TPartNonreplCountersData countersData)
: DiskId(std::move(diskId))
, CountersData(std::move(countersData))
{}
};?
Тогда не надо будет каждый раз конструировать TPartNonreplCountersData:
auto request =
std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
MakeIntrusive<TCallContext>(),
DiskId,
ExtractPartCounters(ctx));
...
UpdateCounters(ctx, ev->Sender, std::move(msg->CountersData));|
|
||
| //////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| struct TPartNonreplCountersData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
М.б. в disk_counters.h положить?
| TPartitionDiskCountersPtr DiskCounters; | ||
| ui64 NetworkBytes; | ||
| TDuration CpuUsage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TPartNonreplCountersData CountersData;
3bab0f0 to
0bfe8a8
Compare
komarevtsev-d
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Надо выпилить seqNo отовсюду
| size_t MultiAgentWriteRoundRobinSeed = 0; | ||
|
|
||
| TRequestInfoPtr StatisticRequestInfo; | ||
| ui64 StatisticSeqNo = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Надо из всех акторов выпилить это поле
|
|
||
| struct TDiskRegistryBasedPartCountersCombined | ||
| { | ||
| const ui64 SeqNo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
И вот это
Issue #3154
This is the second step in the transition from a push scheme to a pull scheme for collecting statistics of partitions.
The first part was about YDB-based disks: #3803
This PR is about DR-based disks.
Added
TDiskRegistryBasedPartitionStatisticsCollectorActor, which is used in DR-based partitions that own multiple other partitions (e.g., mirror partition, migration partition). This actor manages the process of collecting statistics from its children.With the
UsePullSchemeForVolumeStatisticssetting enabled, statistics should be collected faster due to the absence of delays in sending them from the partition.