Handle InferPDiskSlotCount settings in nodewarden #31207

rosik · 2025-12-23T14:59:58Z

Changelog entry

...

Changelog category

Not for changelog (changelog entry is not required)

Description for reviewers

...

github-actions · 2025-12-23T15:01:24Z

⚪ 2025-12-23 15:01:23 UTC Pre-commit check linux-x86_64-relwithdebinfo for 20b5a68 has started.
⚪ 2025-12-23 15:01:39 UTC Artifacts will be uploaded here
⚪ 2025-12-23 15:03:49 UTC ya make is running...
🟡 2025-12-23 17:39:34 UTC Some tests failed, follow the links below. Going to retry failed tests...

Details

Ya make output | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
46883	43303	0	186	3361	33

⚪ 2025-12-23 17:39:51 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-12-23 18:10:04 UTC Some tests failed, follow the links below. Going to retry failed tests...

Details

Ya make output | Test bloat | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
1839 (only retried tests)	1826	0	1	0	12

⚪ 2025-12-23 18:10:11 UTC ya make is running... (failed tests rerun, try 3)
🟢 2025-12-23 18:14:56 UTC Tests successful.

Ya make output | Test bloat | Test bloat | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
1205 (only retried tests)	1194	0	0	0	11

🟢 2025-12-23 18:15:04 UTC Build successful.
🟢 2025-12-23 18:15:30 UTC ydbd size 2.3 GiB changed* by -168.4 KiB, which is <= 0 Bytes vs main: OK

ydbd size dash	main: `54d3846`	merge: `20b5a68`	diff	diff %
ydbd size	2 486 506 360 Bytes	2 486 333 872 Bytes	-168.4 KiB	-0.007%
ydbd stripped size	529 305 024 Bytes	529 300 160 Bytes	-4.8 KiB	-0.001%

^{*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation}

github-actions · 2025-12-23T15:01:32Z

⚪ 2025-12-23 15:01:32 UTC Pre-commit check linux-x86_64-release-asan for 20b5a68 has started.
⚪ 2025-12-23 15:02:14 UTC Artifacts will be uploaded here
⚪ 2025-12-23 15:04:18 UTC ya make is running...
🟡 2025-12-23 16:59:44 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Ya make output | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
15933	15845	0	65	8	15

🟢 2025-12-23 16:59:54 UTC Build successful.
🟢 2025-12-23 17:00:27 UTC ydbd size 3.8 GiB changed* by -205.8 KiB, which is <= 0 Bytes vs main: OK

ydbd size dash	main: `54d3846`	merge: `20b5a68`	diff	diff %
ydbd size	4 086 771 216 Bytes	4 086 560 440 Bytes	-205.8 KiB	-0.005%
ydbd stripped size	1 530 337 880 Bytes	1 530 285 336 Bytes	-51.3 KiB	-0.003%

^{*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation}

ydbot · 2025-12-23T15:02:12Z

Run Extra Tests

Run additional tests for this PR. You can customize:

Test Size: small, medium, large (default: all)
Test Targets: any directory path (default: ydb/)
Sanitizers: ASAN, MSAN, TSAN
Coredumps: enable for debugging (default: off)
Additional args: custom ya make arguments

github-actions · 2025-12-23T15:03:09Z

🟢 2025-12-23 15:03:09 UTC The validation of the Pull Request description is successful.

Copilot

Pull request overview

This PR refactors the InferPDiskSlotCount settings from per-PDisk configuration to a global BlobStorageConfig setting. The implementation enables dynamic reconfiguration through NodeWarden's subscription to configuration updates.

Key Changes

Moved InferPDiskSlotCount settings from individual PDisk configuration to global BlobStorageConfig
Implemented dynamic configuration updates in NodeWarden via config subscription mechanism
Removed deprecated per-PDisk InferPDiskSlotCountFromUnitSize and InferPDiskSlotCountMax fields from schemas and protobuf definitions

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
ydb/core/protos/blobstorage_config.proto	Added new TInferPDiskSlotCountSettings message structure and reserved deprecated fields
ydb/core/protos/blobstorage.proto	Reserved deprecated InferPDiskSlotCountFromUnitSize and InferPDiskSlotCountMax fields in TPDisk message
ydb/core/protos/config.proto	Added InferPDiskSlotCountSettings field to TBlobStorageConfig
ydb/core/protos/sys_view.proto	Reserved deprecated InferPDiskSlotCountFromUnitSize field in TPDiskInfo
ydb/core/blobstorage/pdisk/blobstorage_pdisk_config.h	Added TInferPDiskSlotCountSettingsForDriveType helper struct
ydb/core/blobstorage/nodewarden/node_warden_impl.h	Added InferPDiskSlotCountSettings member and config notification handlers
ydb/core/blobstorage/nodewarden/node_warden_impl.cpp	Implemented config subscription and dynamic settings update in Handle(TEvConfigNotificationRequest)
ydb/core/blobstorage/nodewarden/node_warden_pdisk.cpp	Updated CreatePDiskConfig to use global InferPDiskSlotCountSettings
ydb/core/mind/bscontroller/scheme.h	Commented out deprecated InferPDiskSlotCountFromUnitSize and InferPDiskSlotCountMax columns
ydb/core/mind/bscontroller/impl.h	Updated TPDiskInfo and TDriveInfo structures, removed deprecated fields
ydb/core/mind/bscontroller/*.cpp	Updated various controller methods to remove usage of deprecated fields
ydb/library/yaml_config/protos/config.proto	Reserved deprecated InferPDiskSlotCountFromUnitSize and InferPDiskSlotCountMax fields
ydb/library/yaml_config/yaml_config_parser.cpp	Removed code that propagated per-host InferPDiskSlotCount settings to drives
ydb/core/sys_view/*.cpp	Removed or commented out references to deprecated InferPDiskSlotCountFromUnitSize field
ydb/tests/library/harness/kikimr_config.py	Added infer_pdisk_slot_count_settings parameter to KikimrConfigGenerator
ydb/core/blobstorage/nodewarden/blobstorage_node_warden_ut.cpp	Updated tests to use new config notification API for setting InferPDiskSlotCount
ydb/apps/dstool/lib/common.py	Simplified get_pdisk_inferred_settings by removing deprecated InferPDiskSlotCountFromUnitSize check
ydb/tests/functional/scheme_tests/canondata/*	Updated canonical schema data reflecting removal of deprecated columns
ydb/library/yaml_config/ut_transform/canondata/*	Updated line numbers in error messages due to code removal

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T15:14:55Z

ydb/core/mind/bscontroller/sys_view.cpp

+                // TODO
+                //
+                // bool shouldInfer = inferSettingsForDriveType && (!explicitSlotCount || preferInferred);


The TODO comment appears to be incomplete or outdated. Since the refactoring is complete and the feature is working (as evidenced by tests), this TODO should either be implemented, converted to a more descriptive comment explaining the logic, or removed entirely.

Suggested change

// TODO

//

// bool shouldInfer = inferSettingsForDriveType && (!explicitSlotCount || preferInferred);

// Infer PDisk settings from the collected metrics. Any decisions about

// whether to use inferred vs. explicit configuration (e.g. feature flags)

// are made when building StaticPDisks / global configuration, so here we

// always extract and expose the inferred values.

Copilot · 2025-12-23T15:14:55Z

ydb/core/protos/blobstorage_config.proto

 }

+message TInferPDiskSlotCountForDriveTypeSettings {
+    // NodeWarded infers PDiskConfig ExpectedSlotCount and SlotSizeInUnits by the formula


There's a typo in the comment: "NodeWarded" should be "NodeWarden".

Suggested change

// NodeWarded infers PDiskConfig ExpectedSlotCount and SlotSizeInUnits by the formula

// NodeWarden infers PDiskConfig ExpectedSlotCount and SlotSizeInUnits by the formula

Copilot · 2025-12-23T15:14:55Z

ydb/core/protos/blobstorage_config.proto

+message TInferPDiskSlotCountSettings {
+    optional bool PreferInferredSettingsOverExplicit = 1; // Unless true, PDisk.ExpectedSlotCount overrides inferred settings
+    optional TInferPDiskSlotCountForDriveTypeSettings Rot = 2;
+    optional TInferPDiskSlotCountForDriveTypeSettings Ssd = 3;


The Ssd field should include a comment documenting that it applies to both SSD and NVME device types, similar to the comment that was present in the deprecated proto message definitions.

Suggested change

optional TInferPDiskSlotCountForDriveTypeSettings Ssd = 3;

optional TInferPDiskSlotCountForDriveTypeSettings Ssd = 3; // Applies to both SSD and NVME device types.

Copilot · 2025-12-23T15:14:56Z

ydb/core/mind/bscontroller/impl.h

+            bool settingsShouldBeInferred = !pdisk->HasExpectedSlotCount &&
+                StorageConfig && StorageConfig->HasBlobStorageConfig() &&
+                StorageConfig->GetBlobStorageConfig().HasInferPDiskSlotCountSettings() &&
+                (pdisk->Kind.Type() == NPDisk::DEVICE_TYPE_ROT ?
+                    StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasRot() :
+                    StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasSsd());


The logic for checking if settings should be inferred for SSD/NVME devices may not handle NVME correctly. When the device type is NVME, the check uses HasSsd(), but there's no guarantee that Ssd settings are appropriate for NVME devices. Consider explicitly checking for NVME and using the appropriate settings, or document that Ssd settings apply to both SSD and NVME devices.

Suggested change

bool settingsShouldBeInferred = !pdisk->HasExpectedSlotCount &&

StorageConfig && StorageConfig->HasBlobStorageConfig() &&

StorageConfig->GetBlobStorageConfig().HasInferPDiskSlotCountSettings() &&

(pdisk->Kind.Type() == NPDisk::DEVICE_TYPE_ROT ?

StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasRot() :

StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasSsd());

const bool hasInferSettings =

StorageConfig &&

StorageConfig->HasBlobStorageConfig() &&

StorageConfig->GetBlobStorageConfig().HasInferPDiskSlotCountSettings();

const bool isRotational = pdisk->Kind.Type() == NPDisk::DEVICE_TYPE_ROT;

// NOTE: For non-rotational devices (SSD, NVME, etc.) we intentionally use the "Ssd"

// settings from InferPDiskSlotCountSettings. This is by design: Ssd settings apply

// to all solid-state devices, including NVME.

const bool hasDeviceSpecificSettings = isRotational

? StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasRot()

: StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasSsd();

bool settingsShouldBeInferred = !pdisk->HasExpectedSlotCount &&

hasInferSettings &&

hasDeviceSpecificSettings;

rosik added 2 commits December 23, 2025 17:58

Handle InferPDiskSlotCount settings in nodewarden

39aec02

Fix tests

5db8379

rosik self-assigned this Dec 23, 2025

Copilot AI review requested due to automatic review settings December 23, 2025 14:59

Copilot started reviewing on behalf of rosik December 23, 2025 15:02 View session

github-actions bot added the not-for-changelog label Dec 23, 2025

Copilot AI reviewed Dec 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Handle InferPDiskSlotCount settings in nodewarden #31207

Handle InferPDiskSlotCount settings in nodewarden #31207

Uh oh!

rosik commented Dec 23, 2025

Uh oh!

github-actions bot commented Dec 23, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 23, 2025 •

edited

Loading

Uh oh!

ydbot commented Dec 23, 2025

Uh oh!

github-actions bot commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-                // TODO
-                //
-                // bool shouldInfer = inferSettingsForDriveType && (!explicitSlotCount || preferInferred);
+                // Infer PDisk settings from the collected metrics. Any decisions about
+                // whether to use inferred vs. explicit configuration (e.g. feature flags)
+                // are made when building StaticPDisks / global configuration, so here we
+                // always extract and expose the inferred values.

	// NodeWarded infers PDiskConfig ExpectedSlotCount and SlotSizeInUnits by the formula
	// NodeWarden infers PDiskConfig ExpectedSlotCount and SlotSizeInUnits by the formula

	optional TInferPDiskSlotCountForDriveTypeSettings Ssd = 3;
	optional TInferPDiskSlotCountForDriveTypeSettings Ssd = 3; // Applies to both SSD and NVME device types.

-            bool settingsShouldBeInferred = !pdisk->HasExpectedSlotCount &&
-                StorageConfig && StorageConfig->HasBlobStorageConfig() &&
-                StorageConfig->GetBlobStorageConfig().HasInferPDiskSlotCountSettings() &&
-                (pdisk->Kind.Type() == NPDisk::DEVICE_TYPE_ROT ?
-                    StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasRot() :
-                    StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasSsd());
+            const bool hasInferSettings =
+                StorageConfig &&
+                StorageConfig->HasBlobStorageConfig() &&
+                StorageConfig->GetBlobStorageConfig().HasInferPDiskSlotCountSettings();
+            const bool isRotational = pdisk->Kind.Type() == NPDisk::DEVICE_TYPE_ROT;
+            // NOTE: For non-rotational devices (SSD, NVME, etc.) we intentionally use the "Ssd"
+            // settings from InferPDiskSlotCountSettings. This is by design: Ssd settings apply
+            // to all solid-state devices, including NVME.
+            const bool hasDeviceSpecificSettings = isRotational
+                ? StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasRot()
+                : StorageConfig->GetBlobStorageConfig().GetInferPDiskSlotCountSettings().HasSsd();
+            bool settingsShouldBeInferred = !pdisk->HasExpectedSlotCount &&
+                hasInferSettings &&
+                hasDeviceSpecificSettings;

Handle InferPDiskSlotCount settings in nodewarden #31207

Are you sure you want to change the base?

Handle InferPDiskSlotCount settings in nodewarden #31207

Uh oh!

Conversation

rosik commented Dec 23, 2025

Changelog entry

Changelog category

Description for reviewers

Uh oh!

github-actions bot commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ydbot commented Dec 23, 2025

Run Extra Tests

Uh oh!

github-actions bot commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Dec 23, 2025 •

edited

Loading

github-actions bot commented Dec 23, 2025 •

edited

Loading