Conversation
Run Extra TestsRun additional tests for this PR. You can customize:
|
|
⚪ ⚪ 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 |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
There was a problem hiding this comment.
Pull request overview
Introduces an initial (“skeleton”) host statistics model for partition_direct, and wires it into TDirectBlockGroup to record per-host success/error events and execution time for key operations.
Changes:
- Added new
partition_direct/modellibrary withTHostStat(+ unit test) to track error streak duration/count. - Instrumented
TDirectBlockGroupasync operation completions to call host-stat hooks (OnResponse/OnMultiFlushResponse) with measured execution time. - Renamed
HostId→HostIndexin multi-PBuffer write responses and adjusted consumers; removed an unnecessary constructor body inTFastPathService.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/ya.make | Adds the new model submodule to build and links it into partition_direct. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/write_request.cpp | Consumes renamed HostIndex field when mapping multi-write results back to locations. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/model/ya.make | New library target for the host-stat model implementation. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/model/ut/ya.make | New unit test target wiring for the host-stat model. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/model/host_stat.h | Declares EOperation and THostStat API. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/model/host_stat.cpp | Implements basic error tracking/reset behavior for THostStat. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/model/host_stat_ut.cpp | Unit tests for error duration/count behavior. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/fast_path_service.cpp | Removes now-unneeded ctor body / Y_UNUSED call. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/direct_block_group.h | Adds host-stat member storage and helper methods; updates response struct field name. |
| ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/direct_block_group.cpp | Records per-operation execution time and updates host stats on responses. |
Comments suppressed due to low confidence (1)
ydb/core/nbs/cloud/blockstore/libs/storage/partition_direct/direct_block_group.cpp:514
- In WriteBlocksToManyPBuffers,
disksIdsis constructed withhostIndexes.size()(pre-sized) and then the loop appends withpush_back(). This results in a vector that is larger than intended and contains leading default-initialized TDDiskId entries, soWriteToManyPBuffersmay receive unexpected/invalid destination IDs. PreferReserve(hostIndexes.size())(or default-construct empty) andpush_back, or fill by index withoutpush_back.
TVector<NKikimrBlobStorage::NDDisk::TDDiskId> disksIds(hostIndexes.size());
for (auto hostIndex: hostIndexes) {
const auto& ddiskId =
PBufferConnections[hostIndex].HostConnection.DDiskId;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BackportTo backport this PR, click the button next to the target branch and then click "Run workflow" in the Run Actions UI.
|
Changelog entry
...
Changelog category
Description for reviewers
...