Skip to content

Conversation

@the-ancient-1
Copy link
Member

@the-ancient-1 the-ancient-1 commented May 13, 2025

Changelog entry

...

Changelog category

  • Not for changelog (changelog entry is not required)

Description for reviewers

Close #18217
Close #18218

@github-actions
Copy link

github-actions bot commented May 13, 2025

🟢 2025-05-14 17:21:15 UTC The validation of the Pull Request description is successful.

@github-actions
Copy link

github-actions bot commented May 13, 2025

2025-05-13 23:19:37 UTC Pre-commit check linux-x86_64-relwithdebinfo for fa3e8d3 has started.
2025-05-13 23:19:48 UTC Artifacts will be uploaded here
2025-05-13 23:22:43 UTC ya make is running...
2025-05-13 23:50:08 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented May 13, 2025

2025-05-13 23:19:47 UTC Pre-commit check linux-x86_64-release-asan for fa3e8d3 has started.
2025-05-13 23:19:58 UTC Artifacts will be uploaded here
2025-05-13 23:22:43 UTC ya make is running...
2025-05-13 23:50:07 UTC Check cancelled

@the-ancient-1 the-ancient-1 changed the title Fix PDisk shred free chunks in transit, #18217 Fix PDisk shred free chunks in transit, #18217, #18218 May 13, 2025
@the-ancient-1 the-ancient-1 changed the title Fix PDisk shred free chunks in transit, #18217, #18218 Fix PDisk shred free chunks in transit and after vdisk deletion #18217, #18218 May 13, 2025
@github-actions
Copy link

github-actions bot commented May 13, 2025

2025-05-13 23:54:26 UTC Pre-commit check linux-x86_64-relwithdebinfo for 0652d05 has started.
2025-05-13 23:54:37 UTC Artifacts will be uploaded here
2025-05-13 23:57:22 UTC ya make is running...
🟡 2025-05-14 00:53:18 UTC Some tests failed, follow the links below. Going to retry failed tests...

Details

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
21417 20053 0 3 1326 35

2025-05-14 00:55:05 UTC ya make is running... (failed tests rerun, try 2)
🟢 2025-05-14 01:18:19 UTC Tests successful.

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
561 (only retried tests) 523 0 0 8 30

🟢 2025-05-14 01:18:27 UTC Build successful.
🟡 2025-05-14 01:18:51 UTC ydbd size 2.2 GiB changed* by +739.0 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: 2f964b1 merge: 0652d05 diff diff %
ydbd size 2 352 022 696 Bytes 2 352 779 408 Bytes +739.0 KiB +0.032%
ydbd stripped size 494 258 352 Bytes 494 532 416 Bytes +267.6 KiB +0.055%

*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
Copy link

github-actions bot commented May 14, 2025

2025-05-14 00:11:28 UTC Pre-commit check linux-x86_64-release-asan for 0652d05 has started.
2025-05-14 00:11:56 UTC Artifacts will be uploaded here
2025-05-14 00:15:29 UTC ya make is running...
🟡 2025-05-14 01:41:23 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

Details

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13420 13271 0 76 46 27

2025-05-14 01:42:37 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-05-14 02:17:25 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

Details

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1443 (only retried tests) 1322 0 69 27 25

2025-05-14 02:17:42 UTC ya make is running... (failed tests rerun, try 3)
🟡 2025-05-14 02:49:51 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?
1250 (only retried tests) 1127 0 55 43 25

🟢 2025-05-14 02:50:06 UTC Build successful.
🟡 2025-05-14 02:50:42 UTC ydbd size 3.9 GiB changed* by +1.7 MiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: 2f964b1 merge: 0652d05 diff diff %
ydbd size 4 138 199 192 Bytes 4 139 971 664 Bytes +1.7 MiB +0.043%
ydbd stripped size 1 435 994 752 Bytes 1 437 073 816 Bytes +1.0 MiB +0.075%

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

@rosik rosik changed the title Fix PDisk shred free chunks in transit and after vdisk deletion #18217, #18218 Fix PDisk shred free chunks in transit and after vdisk deletion May 14, 2025
@rosik rosik merged commit 30c27a0 into ydb-platform:main May 14, 2025
20 of 21 checks passed
xyliganSereja pushed a commit to xyliganSereja/ydb_work that referenced this pull request Jun 3, 2025
@the-ancient-1 the-ancient-1 requested a review from Copilot June 6, 2025 20:09
Copy link
Contributor

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 fixes issues with PDisk shredding by ensuring that chunks flagged for deletion are correctly marked as dirty and processed appropriately during state transitions.

  • Marks chunks as dirty during owner kill when shred mode is enabled.
  • Adds a loop to check and schedule shredding for chunks in transition between states.

<< " state# " << state.ToString()
<< ", there are already ContinueShredsInFlight# " << ContinueShredsInFlight.load()
<< " so just wait for it to arrive. "
<< " ShredGeneration# " << ShredGeneration);
Copy link

Copilot AI Jun 6, 2025

Choose a reason for hiding this comment

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

The early return within the loop means that only the first matching dirty chunk is processed per call. If this is intended, please add a comment to clarify that scheduling only one continue shred per invocation is by design.

Suggested change
<< " ShredGeneration# " << ShredGeneration);
<< " ShredGeneration# " << ShredGeneration);
// Early return is intentional: only the first matching dirty chunk is processed per invocation.
// This ensures that scheduling only one continue shred per invocation is by design.

Copilot uses AI. Check for mistakes.
<< ", scheduling ContinueShred. "
<< " ShredGeneration# " << ShredGeneration);
ContinueShredsInFlight++;
PCtx->ActorSystem->Schedule(TDuration::MilliSeconds(50),
Copy link

Copilot AI Jun 6, 2025

Choose a reason for hiding this comment

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

[nitpick] The literal delay value '50' in TDuration::MilliSeconds(50) could be replaced with a named constant to improve readability and maintainability.

Suggested change
PCtx->ActorSystem->Schedule(TDuration::MilliSeconds(50),
PCtx->ActorSystem->Schedule(TDuration::MilliSeconds(CONTINUE_SHRED_DELAY_MS),

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support data cleanup after database deletion Shred chunks after log cleaning

2 participants