minimize returned data to avoid large responses #19878
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🟢 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | 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 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | 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 |
adameat
force-pushed
the
fix-node-network-perf
branch
from
96576f6 to
6b2b627
Compare
adameat
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
adameat
force-pushed
the
fix-node-network-perf
branch
from
6b2b627 to
f49227f
Compare
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | 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 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | 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 |
adameat
force-pushed
the
fix-node-network-perf
branch
from
f49227f to
b47687b
Compare
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | 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 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces selective field rendering to reduce payload sizes by only including data the user explicitly requests, updates handler priorities, and aligns test expectations accordingly.
- Added
FieldsRequestedto mirror user-requested fields and gated JSON output on it - Updated HTTP handler registration order for
/viewer/nodesand/storage/groups - Revised canonical JSON tests to remove or reorganize fields that are no longer always returned
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/core/viewer/viewer_nodes.h | Introduce FieldsRequested, gate per-field JSON output |
| ydb/core/viewer/storage_groups.h | Same selective-rendering logic for storage groups responses |
| ydb/core/viewer/json_handlers_viewer.cpp | Bumped handler priority for viewer nodes endpoint |
| ydb/core/viewer/json_handlers_storage.cpp | Bumped handler priority for storage groups endpoint |
| ydb/core/viewer/tests/canondata/result.json | Updated expected JSON output to match minimized responses |
Comments suppressed due to low confidence (3)
ydb/core/viewer/viewer_nodes.h:1064
- Document in the Swagger spec (in
TJsonNodes::GetSwagger) the newfieldsquery parameter so API consumers know how to request specific fields.
FieldsRequested = FieldsRequired; // no dependent fields
ydb/core/viewer/json_handlers_viewer.cpp:264
- The handler priority was incremented from 15 to 16; verify this change does not inadvertently affect routing order and document the reasoning.
handlers.AddHandler("/viewer/nodes", new TJsonHandler<TJsonNodes>(TJsonNodes::GetSwagger()), 16);
ydb/core/viewer/tests/canondata/result.json:150
- Removed
size_bytesentries from the canonical tests; consider adding dedicated tests that explicitly request and verifysize_bytesbehavior remains correct when requested.
"owner": "user1",
| jsonNode.SetDatabase(node->Database); | ||
| } | ||
| if (node->UptimeSeconds) { | ||
| if (node->UptimeSeconds && FieldsRequested.test(+ENodeFields::Uptime)) { | ||
| jsonNode.SetUptimeSeconds(*(node->UptimeSeconds)); | ||
| } | ||
| if (node->Disconnected) { |
There was a problem hiding this comment.
[nitpick] The Disconnected field is always included; consider gating it with FieldsRequested.test(+ENodeFields::Disconnected) for consistency with other optional fields.
| if (node->Disconnected) { | |
| if (node->Disconnected && FieldsRequested.test(+ENodeFields::Disconnected)) { |
Copilot uses AI. Check for mistakes.
| TFieldsType FieldsRequested; // fields that were requested by user | ||
| TFieldsType FieldsRequired; // fields that are required to calculate the response |
There was a problem hiding this comment.
[nitpick] FieldsRequired and FieldsRequested names are quite similar and can be confusing; consider renaming to DependentFields and UserRequestedFields for clarity.
| TFieldsType FieldsRequested; // fields that were requested by user | |
| TFieldsType FieldsRequired; // fields that are required to calculate the response | |
| TFieldsType UserRequestedFields; // fields that were requested by user | |
| TFieldsType DependentFields; // fields that are required to calculate the response |
Copilot uses AI. Check for mistakes.
| "MaxTableColumns": "200", | ||
| "MaxTableIndices": "20", | ||
| "MaxTableKeyColumns": "30" | ||
| }, | ||
| "SchemeShardId_Depricated": "72057594046678944", |
There was a problem hiding this comment.
The field SchemeShardId_Depricated appears to be misspelled; consider renaming to SchemeShardId_Deprecated to match conventional spelling.
| "SchemeShardId_Depricated": "72057594046678944", | |
| "SchemeShardId_Deprecated": "72057594046678944", |
Copilot uses AI. Check for mistakes.
Changelog entry
return requested data only, minimizes huge responses, especially on network pages, closes #19810
Changelog category
Description for reviewers
...