Skip to content

minimize returned data to avoid large responses #19878

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

Merged
merged 1 commit into from
Jun 20, 2025

Conversation

adameat
Copy link
Member

@adameat adameat commented Jun 19, 2025

Changelog entry

return requested data only, minimizes huge responses, especially on network pages, closes #19810

Changelog category

  • Not for changelog (changelog entry is not required)

Description for reviewers

...

@adameat adameat self-assigned this Jun 19, 2025
@adameat adameat requested a review from a team as a code owner June 19, 2025 08:43
@adameat adameat added the area/ui UI/Embedded UI related issues label Jun 19, 2025
@adameat adameat requested a review from Copilot June 19, 2025 08:43
Copilot

This comment was marked as outdated.

Copy link

github-actions bot commented Jun 19, 2025

🟢 2025-06-20 05:22:35 UTC The validation of the Pull Request description is successful.

Copy link

github-actions bot commented Jun 19, 2025

2025-06-19 08:46:43 UTC Pre-commit check linux-x86_64-relwithdebinfo for ec0e49e has started.
2025-06-19 08:46:55 UTC Artifacts will be uploaded here
2025-06-19 08:50:09 UTC ya make is running...
🟡 2025-06-19 09:50:17 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
28532 27125 0 13 1347 47

2025-06-19 09:52:43 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-06-19 10:10:31 UTC Some tests failed, follow the links below. Going to retry failed tests...

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
515 (only retried tests) 479 0 6 1 29

2025-06-19 10:10:44 UTC ya make is running... (failed tests rerun, try 3)
🔴 2025-06-19 10:21:36 UTC Some tests failed, follow the links below.

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
312 (only retried tests) 277 0 6 0 29

🟢 2025-06-19 10:21:44 UTC Build successful.
🟢 2025-06-19 10:22:05 UTC ydbd size 2.2 GiB changed* by +5.0 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 9543c8e merge: ec0e49e diff diff %
ydbd size 2 375 953 376 Bytes 2 375 958 504 Bytes +5.0 KiB +0.000%
ydbd stripped size 497 918 408 Bytes 497 919 496 Bytes +1.1 KiB +0.000%

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

Copy link

github-actions bot commented Jun 19, 2025

2025-06-19 08:46:49 UTC Pre-commit check linux-x86_64-release-asan for ec0e49e has started.
2025-06-19 08:47:02 UTC Artifacts will be uploaded here
2025-06-19 08:50:23 UTC ya make is running...
🟡 2025-06-19 10:33:28 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13522 13169 0 118 203 32

2025-06-19 10:34:48 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-06-19 11:08:35 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1751 (only retried tests) 1490 0 58 181 22

2025-06-19 11:08:55 UTC ya make is running... (failed tests rerun, try 3)
🟡 2025-06-19 11:39:47 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1433 (only retried tests) 1159 0 61 184 29

🟢 2025-06-19 11:40:03 UTC Build successful.
🟢 2025-06-19 11:40:39 UTC ydbd size 3.9 GiB changed* by +7.3 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 9543c8e merge: ec0e49e diff diff %
ydbd size 4 179 402 344 Bytes 4 179 409 848 Bytes +7.3 KiB +0.000%
ydbd stripped size 1 448 933 496 Bytes 1 448 935 544 Bytes +2.0 KiB +0.000%

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

@adameat adameat force-pushed the fix-node-network-perf branch from 96576f6 to 6b2b627 Compare June 20, 2025 05:19
@adameat adameat changed the title return what requested only minimize returned data to avoid large responses Jun 20, 2025
@adameat adameat enabled auto-merge (squash) June 20, 2025 05:21
Copy link

github-actions bot commented Jun 20, 2025

2025-06-20 05:22:26 UTC Pre-commit check linux-x86_64-release-asan for b7ddd89 has started.
2025-06-20 05:22:37 UTC Artifacts will be uploaded here
2025-06-20 05:25:55 UTC ya make is running...
2025-06-20 05:48:30 UTC Check cancelled

Copy link

github-actions bot commented Jun 20, 2025

2025-06-20 05:22:31 UTC Pre-commit check linux-x86_64-relwithdebinfo for b7ddd89 has started.
2025-06-20 05:22:41 UTC Artifacts will be uploaded here
2025-06-20 05:25:47 UTC ya make is running...
2025-06-20 05:48:28 UTC Check cancelled

@adameat adameat requested review from Copilot and alexvru June 20, 2025 05:35
Copilot

This comment was marked as outdated.

@adameat adameat force-pushed the fix-node-network-perf branch from 6b2b627 to f49227f Compare June 20, 2025 05:48
Copy link

github-actions bot commented Jun 20, 2025

2025-06-20 05:49:59 UTC Pre-commit check linux-x86_64-release-asan for 2033f65 has started.
2025-06-20 05:53:01 UTC Artifacts will be uploaded here
2025-06-20 06:18:11 UTC ya make is running...
🟡 2025-06-20 08:01:34 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13530 13091 0 204 207 28

2025-06-20 08:02:49 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-06-20 08:40:31 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
2227 (only retried tests) 1937 0 84 180 26

2025-06-20 08:40:53 UTC ya make is running... (failed tests rerun, try 3)
🟢 2025-06-20 08:44:15 UTC ydbd size 3.9 GiB changed* by +7.4 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: ecbf05b merge: 2033f65 diff diff %
ydbd size 4 180 099 280 Bytes 4 180 106 832 Bytes +7.4 KiB +0.000%
ydbd stripped size 1 449 133 240 Bytes 1 449 135 352 Bytes +2.1 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation
2025-06-20 08:44:17 UTC Check cancelled

Copy link

github-actions bot commented Jun 20, 2025

2025-06-20 05:52:04 UTC Pre-commit check linux-x86_64-relwithdebinfo for 2033f65 has started.
2025-06-20 05:52:15 UTC Artifacts will be uploaded here
2025-06-20 06:18:03 UTC ya make is running...
🟡 2025-06-20 07:23:17 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
28541 27151 0 5 1348 37

2025-06-20 07:25:40 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-06-20 07:36:34 UTC Some tests failed, follow the links below. Going to retry failed tests...

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
522 (only retried tests) 487 0 2 3 30

2025-06-20 07:36:45 UTC ya make is running... (failed tests rerun, try 3)
🔴 2025-06-20 07:44:46 UTC Some tests failed, follow the links below.

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
267 (only retried tests) 237 0 2 0 28

🟢 2025-06-20 07:44:54 UTC Build successful.
🟢 2025-06-20 07:45:15 UTC ydbd size 2.2 GiB changed* by +5.0 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: ecbf05b merge: 2033f65 diff diff %
ydbd size 2 376 339 392 Bytes 2 376 344 560 Bytes +5.0 KiB +0.000%
ydbd stripped size 497 981 256 Bytes 497 982 408 Bytes +1.1 KiB +0.000%

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

alexvru
alexvru previously approved these changes Jun 20, 2025
Copy link

github-actions bot commented Jun 20, 2025

2025-06-20 08:45:44 UTC Pre-commit check linux-x86_64-release-asan for 1a28217 has started.
2025-06-20 08:46:09 UTC Artifacts will be uploaded here
2025-06-20 08:50:21 UTC ya make is running...
🟡 2025-06-20 10:33:21 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13530 13105 0 194 203 28

2025-06-20 10:34:41 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-06-20 11:13:29 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
2049 (only retried tests) 1776 0 68 180 25

2025-06-20 11:13:49 UTC ya make is running... (failed tests rerun, try 3)
🟡 2025-06-20 11:49:00 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1812 (only retried tests) 1552 0 60 177 23

🟢 2025-06-20 11:49:18 UTC Build successful.
🟢 2025-06-20 11:49:54 UTC ydbd size 3.9 GiB changed* by +7.4 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: f2672d3 merge: 1a28217 diff diff %
ydbd size 4 180 099 296 Bytes 4 180 106 848 Bytes +7.4 KiB +0.000%
ydbd stripped size 1 449 133 240 Bytes 1 449 135 352 Bytes +2.1 KiB +0.000%

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

Copy link

github-actions bot commented Jun 20, 2025

2025-06-20 08:46:15 UTC Pre-commit check linux-x86_64-relwithdebinfo for 1a28217 has started.
2025-06-20 08:46:39 UTC Artifacts will be uploaded here
2025-06-20 08:50:21 UTC ya make is running...
🟡 2025-06-20 09:52:39 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
28541 27145 0 8 1349 39

2025-06-20 09:55:12 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-06-20 10:07:29 UTC Some tests failed, follow the links below. Going to retry failed tests...

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
701 (only retried tests) 627 0 44 4 26

2025-06-20 10:07:40 UTC ya make is running... (failed tests rerun, try 3)
🟢 2025-06-20 10:19:16 UTC Tests successful.

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
429 (only retried tests) 399 0 0 0 30

🟢 2025-06-20 10:19:24 UTC Build successful.
🟢 2025-06-20 10:19:46 UTC ydbd size 2.2 GiB changed* by +5.0 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: f2672d3 merge: 1a28217 diff diff %
ydbd size 2 376 339 424 Bytes 2 376 344 576 Bytes +5.0 KiB +0.000%
ydbd stripped size 497 981 256 Bytes 497 982 408 Bytes +1.1 KiB +0.000%

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

@adameat adameat requested review from Copilot and alexvru June 20, 2025 10:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

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 FieldsRequested to mirror user-requested fields and gated JSON output on it
  • Updated HTTP handler registration order for /viewer/nodes and /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 new fields query 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_bytes entries from the canonical tests; consider adding dedicated tests that explicitly request and verify size_bytes behavior 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) {
Copy link
Preview

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

[nitpick] The Disconnected field is always included; consider gating it with FieldsRequested.test(+ENodeFields::Disconnected) for consistency with other optional fields.

Suggested change
if (node->Disconnected) {
if (node->Disconnected && FieldsRequested.test(+ENodeFields::Disconnected)) {

Copilot uses AI. Check for mistakes.

Comment on lines +626 to +627
TFieldsType FieldsRequested; // fields that were requested by user
TFieldsType FieldsRequired; // fields that are required to calculate the response
Copy link
Preview

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

[nitpick] FieldsRequired and FieldsRequested names are quite similar and can be confusing; consider renaming to DependentFields and UserRequestedFields for clarity.

Suggested change
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",
Copy link
Preview

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

The field SchemeShardId_Depricated appears to be misspelled; consider renaming to SchemeShardId_Deprecated to match conventional spelling.

Suggested change
"SchemeShardId_Depricated": "72057594046678944",
"SchemeShardId_Deprecated": "72057594046678944",

Copilot uses AI. Check for mistakes.

@adameat adameat merged commit ce274a4 into ydb-platform:main Jun 20, 2025
12 checks passed
adameat added a commit to adameat/ydb that referenced this pull request Jun 23, 2025
adameat added a commit to adameat/ydb that referenced this pull request Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui UI/Embedded UI related issues not-for-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bad performance of node list with network information
2 participants