[BugFix] fix concurrent issue between primary index unload and compaction #49341

luohaha · 2024-08-02T10:53:49Z

Why I'm doing:

When Primary Index call unload because of full clone or something else, there could be a background index compaction task was still running. Here is a concurrency safe issue.

What I'm doing:

Check if all input l2 has been removed. If not, that means index has been rebuilt, we need to abort this index compaction task,
Using std::shared_ptr instead of std::unique_ptr, so running index compaction task could finish safely.

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
This is a backport pr

Bugfix cherry-pick branch check:

…tion Signed-off-by: luohaha <[email protected]>

Signed-off-by: luohaha <[email protected]>

github-actions · 2024-08-02T16:30:12Z

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2024-08-02T16:40:26Z

[BE Incremental Coverage Report]

✅ pass : 20 / 20 (100.00%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	be/src/storage/persistent_index.cpp	7	7	100.00%	[]
🔵	be/src/storage/lake/lake_primary_index.cpp	3	3	100.00%	[]
🔵	be/src/storage/primary_index.cpp	10	10	100.00%	[]

kevincai · 2024-08-02T22:44:23Z

be/src/storage/primary_index.cpp


-    if (tablet->get_enable_persistent_index() && (fix_size <= 128)) {


is this a new function change to open the pk index limitation, not related to the bug fix?

It is a historical problem, no need for now.

kevincai · 2024-08-02T23:02:49Z

be/src/storage/lake/lake_primary_index.cpp

@@ -103,13 +102,13 @@ Status LakePrimaryIndex::_do_lake_load(TabletManager* tablet_mgr, const TabletMe
            RETURN_IF_ERROR(StorageEngine::instance()
                                    ->get_persistent_index_store(metadata->id())
                                    ->create_dir_if_path_not_exists(path));
-            _persistent_index = std::make_unique<LakeLocalPersistentIndex>(path);
+            _persistent_index = std::make_shared<LakeLocalPersistentIndex>(path);


is there a necessity to perform the load under lock? since there is already a lock defined and used for pk index.

It will be call when first time initial, no concurrency issue at this phase.

github-actions · 2024-08-06T06:00:31Z

@Mergifyio backport branch-3.3

github-actions · 2024-08-06T06:00:33Z

@Mergifyio backport branch-3.2

github-actions · 2024-08-06T06:00:34Z

@Mergifyio backport branch-3.1

mergify · 2024-08-06T06:00:38Z

backport branch-3.3

✅ Backports have been created

#49426 [BugFix] fix concurrent issue between primary index unload and compaction (backport #49341) has been created for branch branch-3.3

mergify · 2024-08-06T06:00:39Z

backport branch-3.2

✅ Backports have been created

#49427 [BugFix] fix concurrent issue between primary index unload and compaction (backport #49341) has been created for branch branch-3.2 but encountered conflicts

mergify · 2024-08-06T06:00:40Z

backport branch-3.1

✅ Backports have been created

#49428 [BugFix] fix concurrent issue between primary index unload and compaction (backport #49341) has been created for branch branch-3.1 but encountered conflicts

…tion (#49341) Signed-off-by: luohaha <[email protected]> (cherry picked from commit 1c8df3a)

…tion (#49341) Signed-off-by: luohaha <[email protected]> (cherry picked from commit 1c8df3a) # Conflicts: # be/src/storage/lake/lake_primary_index.cpp # be/src/storage/primary_index.cpp # be/test/storage/lake/local_pk_index_manager_test.cpp

…tion (#49341) Signed-off-by: luohaha <[email protected]> (cherry picked from commit 1c8df3a) # Conflicts: # be/src/storage/lake/lake_primary_index.cpp # be/src/storage/persistent_index.h

…tion (backport #49341) (#49427) Signed-off-by: Yixin Luo <[email protected]> Co-authored-by: Yixin Luo <[email protected]>

…tion (backport #49341) (#49428) Signed-off-by: Yixin Luo <[email protected]> Co-authored-by: Yixin Luo <[email protected]>

…tion (backport #49341) (#49426) Co-authored-by: Yixin Luo <[email protected]>

luohaha requested review from a team as code owners August 2, 2024 10:53

mergify bot assigned luohaha Aug 2, 2024

github-actions bot added 3.3 3.2 3.1 labels Aug 2, 2024

[BugFix] fix concurrent issue between primary index unload and compac…

4978ddd

…tion Signed-off-by: luohaha <[email protected]>

luohaha force-pushed the lake-pk-index-bulk-load branch from f0efeee to 4978ddd Compare August 2, 2024 14:08

update

8418353

Signed-off-by: luohaha <[email protected]>

kevincai reviewed Aug 2, 2024

View reviewed changes

kevincai approved these changes Aug 6, 2024

View reviewed changes

decster approved these changes Aug 6, 2024

View reviewed changes

wyb approved these changes Aug 6, 2024

View reviewed changes

wyb merged commit 1c8df3a into StarRocks:main Aug 6, 2024
60 checks passed

github-actions bot removed the 3.3 label Aug 6, 2024

github-actions bot removed the 3.2 label Aug 6, 2024

github-actions bot removed the 3.1 label Aug 6, 2024

mergify bot pushed a commit that referenced this pull request Aug 6, 2024

[BugFix] fix concurrent issue between primary index unload and compac…

700e12a

…tion (#49341) Signed-off-by: luohaha <[email protected]> (cherry picked from commit 1c8df3a)

mergify bot mentioned this pull request Aug 6, 2024

[BugFix] fix concurrent issue between primary index unload and compaction (backport #49341) #49426

Merged

42 tasks

This was referenced Aug 6, 2024

[BugFix] fix concurrent issue between primary index unload and compaction (backport #49341) #49427

Merged

[BugFix] fix concurrent issue between primary index unload and compaction (backport #49341) #49428

Merged

wanpengfei-git pushed a commit that referenced this pull request Aug 6, 2024

[BugFix] fix concurrent issue between primary index unload and compac…

80e79c1

…tion (backport #49341) (#49427) Signed-off-by: Yixin Luo <[email protected]> Co-authored-by: Yixin Luo <[email protected]>

github-actions bot added the 3.2-merged label Aug 6, 2024

wanpengfei-git pushed a commit that referenced this pull request Aug 6, 2024

[BugFix] fix concurrent issue between primary index unload and compac…

f7270e9

…tion (backport #49341) (#49428) Signed-off-by: Yixin Luo <[email protected]> Co-authored-by: Yixin Luo <[email protected]>

github-actions bot added the 3.1-merged label Aug 6, 2024

wanpengfei-git pushed a commit that referenced this pull request Aug 8, 2024

[BugFix] fix concurrent issue between primary index unload and compac…

a9c0c1f

…tion (backport #49341) (#49426) Co-authored-by: Yixin Luo <[email protected]>

github-actions bot added the 3.3-merged label Aug 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BugFix] fix concurrent issue between primary index unload and compaction #49341

[BugFix] fix concurrent issue between primary index unload and compaction #49341

luohaha commented Aug 2, 2024 •

edited

Loading

github-actions bot commented Aug 2, 2024

github-actions bot commented Aug 2, 2024

kevincai Aug 2, 2024

luohaha Aug 6, 2024

kevincai Aug 2, 2024

luohaha Aug 6, 2024 •

edited

Loading

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

mergify bot commented Aug 6, 2024 •

edited

Loading

mergify bot commented Aug 6, 2024 •

edited

Loading

mergify bot commented Aug 6, 2024 •

edited

Loading


		if (tablet->get_enable_persistent_index() && (fix_size <= 128)) {

[BugFix] fix concurrent issue between primary index unload and compaction #49341

[BugFix] fix concurrent issue between primary index unload and compaction #49341

Conversation

luohaha commented Aug 2, 2024 • edited Loading

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

github-actions bot commented Aug 2, 2024

[FE Incremental Coverage Report]

github-actions bot commented Aug 2, 2024

[BE Incremental Coverage Report]

file detail

kevincai Aug 2, 2024

Choose a reason for hiding this comment

luohaha Aug 6, 2024

Choose a reason for hiding this comment

kevincai Aug 2, 2024

Choose a reason for hiding this comment

luohaha Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

mergify bot commented Aug 6, 2024 • edited Loading

✅ Backports have been created

mergify bot commented Aug 6, 2024 • edited Loading

✅ Backports have been created

mergify bot commented Aug 6, 2024 • edited Loading

✅ Backports have been created

luohaha commented Aug 2, 2024 •

edited

Loading

luohaha Aug 6, 2024 •

edited

Loading

mergify bot commented Aug 6, 2024 •

edited

Loading

mergify bot commented Aug 6, 2024 •

edited

Loading

mergify bot commented Aug 6, 2024 •

edited

Loading