Skip to content
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

Index build: do not lose the requested partitioning info of indexImplTables in case of SchemeShard reboots #10579

Merged
merged 3 commits into from
Oct 21, 2024

Conversation

jepett0
Copy link
Collaborator

@jepett0 jepett0 commented Oct 17, 2024

Ticket:

Context

Index building is a long-running operation that is not a single transaction, but many. YDB does persist the state of the individual transactions (i.e. SchemeShard's proposals) and is able to continue them from the last state even if the SchemeShard reboots during the execution of the transaction. However, index build operations require their own persistent description for the SchemeShard to continue them if it reboots.

Description

PersistCreateBuildIndex is a function that is responsible for the index build operation ability to recover from the SchemeShard's reboots. As you can see, it didn't persist the ImplTableDescriptions (they store the partitioning info about the indexImplTables to be created) as well as the Specialized Index Description (i.e. specific vector index description). Previously, if the SchemeShard had rebooted before the indexImplTable was created, but after the index build operation had started, then the partitioning info of the indexImplTables would have been lost. A unit test Is added to reproduce the scenario.

Note

The main commit is:

The others are supplementary (they add unit tests).

This comment was marked as outdated.

This comment was marked as outdated.

@jepett0 jepett0 force-pushed the IndexBackupRestore.persistence.1 branch from 37f46db to 53c0ea3 Compare October 18, 2024 07:56

This comment was marked as outdated.

This comment was marked as outdated.

@jepett0 jepett0 force-pushed the IndexBackupRestore.persistence.1 branch from 53c0ea3 to 6082c72 Compare October 18, 2024 08:43

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

@jepett0 jepett0 marked this pull request as ready for review October 18, 2024 10:55
@jepett0 jepett0 changed the title IndexBackupRestore.persistence.1 Do not lose the requested partitioning info of indexImplTables in case of SchemeShard reboots Oct 18, 2024
@jepett0 jepett0 changed the title Do not lose the requested partitioning info of indexImplTables in case of SchemeShard reboots Index build: do not lose the requested partitioning info of indexImplTables in case of SchemeShard reboots Oct 18, 2024
@jepett0 jepett0 requested a review from MBkkt October 18, 2024 12:03

This comment was marked as outdated.

This comment was marked as outdated.

MBkkt
MBkkt previously approved these changes Oct 18, 2024
ijon
ijon previously approved these changes Oct 18, 2024
@jepett0 jepett0 dismissed stale reviews from ijon and MBkkt via f06fc17 October 18, 2024 12:49

This comment was marked as outdated.

This comment was marked as outdated.

@jepett0 jepett0 force-pushed the IndexBackupRestore.persistence.1 branch from f06fc17 to 270c3df Compare October 18, 2024 16:43
Copy link

github-actions bot commented Oct 18, 2024

2024-10-18 16:45:48 UTC Pre-commit check linux-x86_64-relwithdebinfo for 5dea000 has started.
2024-10-18 16:45:58 UTC Artifacts will be uploaded here
2024-10-18 16:48:58 UTC ya make is running...
🟡 2024-10-18 18:10:02 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?
40319 34555 0 4 5727 33

2024-10-18 18:13:27 UTC ya make is running... (failed tests rerun, try 2)
🟢 2024-10-18 18:24:57 UTC Tests successful.

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
269 (only retried tests) 171 0 0 7 91

🟢 2024-10-18 18:25:05 UTC Build successful.
🟢 2024-10-18 18:25:24 UTC ydbd size 2.8 GiB changed* by +72.9 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: f5d549e merge: 5dea000 diff diff %
ydbd size 3 013 067 224 Bytes 3 013 141 896 Bytes +72.9 KiB +0.002%
ydbd stripped size 477 721 976 Bytes 477 730 072 Bytes +7.9 KiB +0.002%

*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 Oct 18, 2024

2024-10-18 16:46:25 UTC Pre-commit check linux-x86_64-release-asan for 5dea000 has started.
2024-10-18 16:46:37 UTC Artifacts will be uploaded here
2024-10-18 16:49:35 UTC ya make is running...
🟡 2024-10-18 18:39:44 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
10580 10337 0 77 59 107

🟢 2024-10-18 18:40:40 UTC Build successful.
🟡 2024-10-18 18:41:14 UTC ydbd size 5.7 GiB changed* by +120.9 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: f5d549e merge: 5dea000 diff diff %
ydbd size 6 104 513 032 Bytes 6 104 636 824 Bytes +120.9 KiB +0.002%
ydbd stripped size 1 521 867 664 Bytes 1 521 882 192 Bytes +14.2 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

@jepett0 jepett0 merged commit 5c4d7fa into ydb-platform:main Oct 21, 2024
11 checks passed
san-kir-k pushed a commit to spaikus/ydb that referenced this pull request Oct 21, 2024
jepett0 added a commit that referenced this pull request Oct 24, 2024
… an indexImplTable in case of SchemeShard reboots (#10579) (#10634)
uzhastik pushed a commit to uzhastik/ydb that referenced this pull request Oct 24, 2024
jepett0 added a commit to jepett0/ydb that referenced this pull request Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants