Skip to content

Conversation

@PeshkovMikhail
Copy link
Contributor

@PeshkovMikhail PeshkovMikhail commented Dec 8, 2025

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 UsePullSchemeForVolumeStatistics setting enabled, statistics should be collected faster due to the absence of delays in sending them from the partition.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2025

Hi! Thank you for contributing!
The tests on this PR will run after a maintainer adds an ok-to-test label to this PR manually. Thank you for your patience!

@komarevtsev-d komarevtsev-d added blockstore Add this label to run only cloud/blockstore build and tests on PR large-tests Launch large tests for PR ok-to-test Label to approve test launch for external members labels Dec 8, 2025
@github-actions github-actions bot removed the ok-to-test Label to approve test launch for external members label Dec 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 2769c46.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
6006 6005 0 0 0 1 0

Copy link
Collaborator

@qkrorlqr qkrorlqr left a 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

@PeshkovMikhail PeshkovMikhail changed the title From Georg-0001/3930 Pull scheme for non replicated disks Dec 8, 2025
@PeshkovMikhail PeshkovMikhail changed the title Pull scheme for non replicated disks [NBS] Pull scheme for non replicated disks Dec 10, 2025
komarevtsev-d
komarevtsev-d previously approved these changes Dec 14, 2025
Copy link
Collaborator

@komarevtsev-d komarevtsev-d left a 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) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

В каком случае могут перепутаться запросы?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Сейчас это больше перестраховка.
Если ответа на запрос статистики не поступило за 15 секунд, а при перепосылке запроса поступил какой-то ответ, то мы хотим быть уверены, что это новый, а не какой-то из прошлого

Copy link
Collaborator

Choose a reason for hiding this comment

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

А точно нужна эта перестраховка? Сообщения локальные, в одном процессе.

Copy link
Collaborator

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);
Copy link
Collaborator

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
Copy link
Collaborator

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

Comment on lines 61 to 66
std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
MakeIntrusive<TCallContext>(),
std::move(diskCounters),
PartConfig->GetName(),
networkBytes,
cpuUsage);
Copy link
Collaborator

@sharpeye sharpeye Dec 15, 2025

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));

c882926


////////////////////////////////////////////////////////////////////////////////

struct TPartNonreplCountersData
Copy link
Collaborator

Choose a reason for hiding this comment

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

М.б. в disk_counters.h положить?

Comment on lines 404 to 406
TPartitionDiskCountersPtr DiskCounters;
ui64 NetworkBytes;
TDuration CpuUsage;
Copy link
Collaborator

Choose a reason for hiding this comment

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

TPartNonreplCountersData CountersData;

@komarevtsev-d komarevtsev-d added ok-to-test Label to approve test launch for external members recheck Add this label to relaunch checks, it will be automatically removed labels Dec 19, 2025
@github-actions github-actions bot removed ok-to-test Label to approve test launch for external members recheck Add this label to relaunch checks, it will be automatically removed labels Dec 19, 2025
@PeshkovMikhail PeshkovMikhail force-pushed the new-pull-scheme-nonrepl branch from 3bab0f0 to 0bfe8a8 Compare December 19, 2025 08:27
Copy link
Collaborator

@komarevtsev-d komarevtsev-d left a 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;
Copy link
Collaborator

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

И вот это

@github-actions
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit a153553.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
6076 6075 0 0 0 1 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blockstore Add this label to run only cloud/blockstore build and tests on PR large-tests Launch large tests for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants