-
Notifications
You must be signed in to change notification settings - Fork 606
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
Disable merges for indexImplTables partitions when build is in progress #10166
Disable merges for indexImplTables partitions when build is in progress #10166
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
650734b
to
3180d5d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@@ -361,6 +365,7 @@ namespace NSchemeShardUT_Private { | |||
NKikimrSchemeOp::EIndexType IndexType = NKikimrSchemeOp::EIndexTypeGlobal; | |||
TVector<TString> IndexColumns; | |||
TVector<TString> DataColumns; | |||
TVector<NYdb::NTable::TGlobalIndexSettings> GlobalIndexSettings = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, evaluate the following design choice.
I have decided to use a structure defined in the public API to describe a TBuildIndexConfig in SchemeShard's unit tests.
Pros:
- We don't have to repeat the same code in the unit test helpers.
Cons:
- Using public API for unit test helpers might limit us greatly. We would have to change the public description of an index every time we decide to test some new functionality in the SchemeShard tests.
- Counterargument: ut helpers already use a protobuf from the public API to describe an index to be created. It is only natural to use the SDK to work conveniently with the public protobuf. The dependency on the public API and the limitations that come with it would not be lifted, if we didn't use SDK here.
33cb433
to
ccb3ff4
Compare
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Ticket:
Description of the change
Do not merge partitions of indexImplTables if they are not finished being built (i.e. the state of the index object is not equal to EIndexStateReady).
Context
We have implemented saving split boundaries of indexImplTables to the backup recently:
However, the indexImplTable is created empty during the process of an import from S3. The data is being added to it at the filling stage of the BuildIndexes phase of the import process. If there are a lot of partitions in the index, it would take a lot of time to fill them. During the filling stage, SchemeShard might decide to merge the partitions of this indexImplTable. We would like to prevent it, because it will slow down the import process significantly. DataShards cannot build index if its partitions are being merged and would have to wait for the merge to end.
Notes
Commits are separated for your convince: