make deep store upload retry async with configurable parallelism #12017
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #12017 +/- ##
============================================
- Coverage 61.61% 61.49% -0.13%
- Complexity 207 1153 +946
============================================
Files 2385 2389 +4
Lines 129214 129845 +631
Branches 20003 20086 +83
============================================
+ Hits 79613 79845 +232
- Misses 43801 44175 +374
- Partials 5800 5825 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Jackie-Jiang
added
enhancement
release-notes
| @@ -914,9 +915,9 @@ public void testCommitSegmentMetadata() { | |||
| /** | |||
| * Test cases for fixing LLC segment by uploading to segment store if missing | |||
| */ | |||
| @Test | |||
| @Test(timeOut = 30_000L) | |||
There was a problem hiding this comment.
Do not use timeout. We usually use TestUtils.waitForCondition()
...java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManagerTest.java
Outdated
Show resolved
Hide resolved
...ain/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
Outdated
Show resolved
Hide resolved
Jackie-Jiang
approved these changes
Nov 28, 2023
...java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManagerTest.java
Outdated
Show resolved
Hide resolved
...java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManagerTest.java
Outdated
Show resolved
Hide resolved
...ain/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
Outdated
Show resolved
Hide resolved
|
@Jackie-Jiang Thanks for the reviews! addressed the remaining comments |
saurabhd336
pushed a commit
to saurabhd336/pinot
that referenced
this pull request
Feb 9, 2024
…che#12017) * make deep store upload retry async with configurable parallelism * do not queue additional tasks if a task already exists in the queue * address comments, fix metrics * missed cherry-pick, use thread safe set * lint * address comments
wirybeaver
pushed a commit
to wirybeaver/pinot
that referenced
this pull request
Apr 5, 2024
…rallelism (apache#12017) Summary: Cherry-picking https://github.com/apache/pinot/pull/12017/files onto release-1.0.x branch Reviewers: #ldap_rta-group, peck Reviewed By: #ldap_rta-group, peck JIRA Issues: EVA-6295 Differential Revision: https://code.uberinternal.com/D12957267
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to: #11874
This PR moves the deep store upload retry to be an asynchronous operation. By default, the thread pool that performs the uploads is created with a single thread.
A new config is introduced to allow for increasing the size of the thread pool used for retrying segment uploads:
controller.realtime.segment.deepStoreUploadRetry.parallelism(default is 1).A new metric is emitted to track the upload retry queue size, which can be used to ensure we can have visibility into scenarios where segment upload retry cannot keep up with new segments that are missing their deepstore copy:
LLC_SEGMENTS_DEEP_STORE_UPLOAD_RETRY_QUEUE_SIZETested via cluster deployment, edited ZK to remove segment urls to trigger uploads and verified segments were uploaded.
tags:
enhancement