Skip to content

EXT-1527 Provide the new setting RestoreDataS3ReadBatchSize and use it in test ImportBigEncryptedFileTest::ImportBigEncryptedFile to improve test running time #25547

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

EXT-1527 Provide the new setting RestoreDataS3ReadBatchSize and use it in test ImportBigEncryptedFileTest::ImportBigEncryptedFile to improve test running time #25547

wants to merge 1 commit into from
+94 −29

Conversation

UgnineSirdis
Copy link
Collaborator

@UgnineSirdis UgnineSirdis commented Sep 22, 2025 •
edited
Loading

Changelog entry

...

Changelog category

  • Not for changelog (changelog entry is not required)

Description for reviewers

The test ImportBigEncryptedFileTest::ImportBigEncryptedFile timeouts under ASAN because it is quite big. I made its size lower with the help of the new setting RestoreDataS3ReadBatchSize, which allows to setup read parts size during datashard restore.

@UgnineSirdis UgnineSirdis changed the title Increase wait time for test ImportBigEncryptedFileTest::ImportBigEncryptedFile EXT-1527 Increase wait time for test ImportBigEncryptedFileTest::ImportBigEncryptedFile Sep 22, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 22, 2025 •
edited
Loading

🟢 2025-10-16 14:00:20 UTC The validation of the Pull Request description is successful.

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@UgnineSirdis UgnineSirdis force-pushed the increase-test-wait-time branch from a2e616c to 81b78f7 Compare October 16, 2025 13:55
@UgnineSirdis UgnineSirdis requested review from a team as code owners October 16, 2025 13:55
@UgnineSirdis UgnineSirdis changed the title EXT-1527 Increase wait time for test ImportBigEncryptedFileTest::ImportBigEncryptedFile EXT-1527 Provide the new setting RestoreDataS3ReadBatchSize and use it in tests Oct 16, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 16, 2025 •
edited
Loading

⚪ 2025-10-16 13:57:09 UTC Pre-commit check linux-x86_64-release-asan for 4e0a308 has started.
⚪ 2025-10-16 13:57:33 UTC Artifacts will be uploaded here
⚪ 2025-10-16 14:02:13 UTC ya make is running...
🟡 2025-10-16 16:47:05 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?
18017 17569 0 156 270 22

🟢 2025-10-16 16:47:11 UTC Build successful.
🟢 2025-10-16 16:47:34 UTC ydbd size 3.8 GiB changed* by +4.3 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 9196cff merge: 4e0a308 diff diff %
ydbd size 4 033 344 840 Bytes 4 033 349 208 Bytes +4.3 KiB +0.000%
ydbd stripped size 1 498 040 608 Bytes 1 498 042 592 Bytes +1.9 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

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 16, 2025 •
edited
Loading

⚪ 2025-10-16 13:58:57 UTC Pre-commit check linux-x86_64-relwithdebinfo for 4e0a308 has started.
⚪ 2025-10-16 13:59:11 UTC Artifacts will be uploaded here
⚪ 2025-10-16 14:03:16 UTC ya make is running...
🟡 2025-10-16 16:18:05 UTC Some tests failed, follow the links below. Going to retry failed tests...

Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
41238 38391 0 48 2768 31

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

Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
971 (only retried tests) 892 0 42 15 22

⚪ 2025-10-16 16:39:47 UTC ya make is running... (failed tests rerun, try 3)
🔴 2025-10-16 16:58:26 UTC Some tests failed, follow the links below.

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
812 (only retried tests) 733 0 43 17 19

🟢 2025-10-16 16:58:29 UTC Build successful.
🟢 2025-10-16 16:58:48 UTC ydbd size 2.3 GiB changed* by +2.6 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: b84ab08 merge: 4e0a308 diff diff %
ydbd size 2 424 211 720 Bytes 2 424 214 344 Bytes +2.6 KiB +0.000%
ydbd stripped size 516 073 096 Bytes 516 073 736 Bytes +640 Bytes +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

@UgnineSirdis UgnineSirdis changed the title EXT-1527 Provide the new setting RestoreDataS3ReadBatchSize and use it in tests EXT-1527 Provide the new setting RestoreDataS3ReadBatchSize and use it in test ImportBigEncryptedFileTest::ImportBigEncryptedFile to improve test running time Oct 16, 2025
CyberROFL
CyberROFL requested changes Oct 16, 2025
View reviewed changes
ydb/core/protos/datashard_config.proto
optional uint64 InMemoryStateMigrationTimeoutMs = 24 [default = 1000];
optional uint32 StatsReportIntervalSeconds = 25 [default = 10];
optional uint32 BackupTableStatsReportIntervalSeconds = 26 [default = 30];
optional uint32 RestoreDataS3ReadBatchSize = 27 [default = 8388608]; // 8 MB
Copy link
Member

Choose a reason for hiding this comment

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

ydb/ydb/core/protos/s3_settings.proto

Line 45 in 9d32e6d

optional uint32 ReadBatchSize = 1 [default = 8388608]; // 8 MB

Почему это не подходит?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

А это нигде не настраивается. Это исключительно внутренняя настройка. Её нельзя например задать из API. Поэтому сделал, чтобы можно было дефолт менять из конфига. Можно пробросить и в API. В таком случае удобнее будет тестировать конечно, но я не уверен, что управления такими вещами юзером - хорошая идея

Copy link
Member

Choose a reason for hiding this comment

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

Если я правильно понял, сейчас это нужно для тестов. В тестах это используется так:

ydb/ydb/core/tx/schemeshard/ut_restore/ut_restore.cpp

Line 464 in 20f33d5

ReadBatchSize: %d

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

Reviewers

@CyberROFL CyberROFL CyberROFL requested changes

@jepett0 jepett0 jepett0 approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

not-for-changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@UgnineSirdis @CyberROFL @jepett0