Skip to content

Conversation

@Enjection
Copy link
Member

Changelog entry

...

Changelog category

  • New feature
  • Experimental feature
  • Improvement
  • Performance improvement
  • User Interface
  • Bugfix
  • Backward incompatible change
  • Documentation (changelog entry is not required)
  • Not for changelog (changelog entry is not required)

Description for reviewers

...

@github-actions
Copy link

github-actions bot commented Nov 8, 2025

🟢 2025-11-08 02:06:28 UTC The validation of the Pull Request description is successful.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Enjection Enjection force-pushed the feature/incr-backup/indexes-support-003-fixes branch from 7f3bc9a to f0a04ef Compare November 8, 2025 23:04
@github-actions
Copy link

github-actions bot commented Nov 8, 2025

2025-11-08 23:06:32 UTC Pre-commit check linux-x86_64-release-asan for 39d650c has started.
2025-11-08 23:06:50 UTC Artifacts will be uploaded here
2025-11-08 23:09:04 UTC ya make is running...
🟡 2025-11-09 01:06:32 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?
15664 15281 0 138 221 24

🟢 2025-11-09 01:06:46 UTC Build successful.
🟢 2025-11-09 01:07:13 UTC ydbd size 3.8 GiB changed* by +32.2 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 61c53ab merge: 39d650c diff diff %
ydbd size 4 079 105 736 Bytes 4 079 138 760 Bytes +32.2 KiB +0.001%
ydbd stripped size 1 513 384 136 Bytes 1 513 395 016 Bytes +10.6 KiB +0.001%

*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 Nov 8, 2025

2025-11-08 23:06:41 UTC Pre-commit check linux-x86_64-relwithdebinfo for 39d650c has started.
2025-11-08 23:06:59 UTC Artifacts will be uploaded here
2025-11-08 23:09:16 UTC ya make is running...
🟡 2025-11-09 00:46:15 UTC Some tests failed, follow the links below. Going to retry failed tests...

Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
39267 36432 0 6 2803 26

2025-11-09 00:46:34 UTC ya make is running... (failed tests rerun, try 2)
🟢 2025-11-09 00:58:41 UTC Tests successful.

Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
79 (only retried tests) 62 0 0 0 17

🟢 2025-11-09 00:58:49 UTC Build successful.
🟢 2025-11-09 00:59:09 UTC ydbd size 2.3 GiB changed* by +21.7 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 61c53ab merge: 39d650c diff diff %
ydbd size 2 436 051 328 Bytes 2 436 073 536 Bytes +21.7 KiB +0.001%
ydbd stripped size 517 306 128 Bytes 517 311 568 Bytes +5.3 KiB +0.001%

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

@Enjection Enjection marked this pull request as ready for review November 9, 2025 08:19
@Enjection Enjection requested a review from a team as a code owner November 9, 2025 08:19
Copilot AI review requested due to automatic review settings November 9, 2025 08:19
@Enjection Enjection requested a review from a team as a code owner November 9, 2025 08:19
@Enjection Enjection requested a review from CyberROFL November 9, 2025 08:19
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 refactors and enhances the version synchronization logic for continuous backup CDC streams, particularly for tables with indexes. The refactoring extracts inline logic into separate functions and adds support for syncing index entity versions during continuous backup operations.

  • Introduces a new PersistTableIndexAlterVersion function to persist index alter versions to the database
  • Refactors version synchronization logic into dedicated helper functions with improved structure and logging
  • Adds a test case for CDC version synchronization with multiple tables and indexes

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
ydb/core/tx/schemeshard/schemeshard_impl.h Declares new PersistTableIndexAlterVersion function for persisting index alter versions
ydb/core/tx/schemeshard/schemeshard_impl.cpp Implements the new persistence function for table index alter versions
ydb/core/tx/schemeshard/schemeshard__operation_common_cdc_stream.cpp Refactors version synchronization logic into helper functions and adds support for syncing child indexes during continuous backup operations
ydb/core/tx/datashard/datashard_ut_incremental_backup.cpp Adds a new test case CdcVersionSync for testing continuous backup with multiple indexed tables

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +208 to +209
if (versionCtx.IsPartOfContinuousBackup && versionCtx.IsIndexImplTable &&
versionCtx.GrandParentPathId && context.SS->Tables.contains(versionCtx.GrandParentPathId)) {
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

[nitpick] The condition on line 208-209 is complex with multiple boolean checks. Consider extracting this into a descriptive helper function like ShouldSyncWithParentTable to improve readability and maintainability.

Copilot uses AI. Check for mistakes.
@Enjection Enjection merged commit ecf9c0d into main Nov 10, 2025
20 checks passed
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.

4 participants