-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Minion task progress tracking improvements #14521
Open
shounakmk219
wants to merge
6
commits into
apache:master
Choose a base branch
from
shounakmk219:minion-task-observer-enhancements
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Minion task progress tracking improvements #14521
shounakmk219
wants to merge
6
commits into
apache:master
from
shounakmk219:minion-task-observer-enhancements
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: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14521 +/- ##
============================================
+ Coverage 61.75% 63.76% +2.01%
- Complexity 207 1612 +1405
============================================
Files 2436 2711 +275
Lines 133233 151427 +18194
Branches 20636 23365 +2729
============================================
+ Hits 82274 96558 +14284
- Misses 44911 47624 +2713
- Partials 6048 7245 +1197
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
shounakmk219
force-pushed
the
minion-task-observer-enhancements
branch
from
December 26, 2024 04:08
4968917
to
073919d
Compare
shounakmk219
force-pushed
the
minion-task-observer-enhancements
branch
from
January 15, 2025 05:44
c180906
to
b064d6e
Compare
shounakmk219
changed the title
[WIP] Minion task observability refactors
Minion task progress tracking improvements
Jan 15, 2025
shounakmk219
added
enhancement
Configuration
Config changes (addition/deletion/change in behavior)
refactor
observability
minion
labels
Jan 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Configuration
Config changes (addition/deletion/change in behavior)
enhancement
minion
observability
refactor
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.
Description
This PR tries to decouple the way how minion task progress is being tracked and stored so that we can reuse the implementations of
MinionEventObserver
and decide the underlying storage to be used based on the requirements.This PR also adds some extra stats to track as part of the observer so that user can get more granular information about the minion task execution.
Introduce new
MinionTaskProgressManager
interfaceThe responsibility of storing the task progress data that's being monitored by
MinionEventObserver
will be delegated to this new interfaceMinionTaskProgressManager
. It will expose the below methods to manage theMinionTaskProgressStats
for each task run.init(PinotConfiguration configuration)
getTaskProgress(String taskId)
setTaskProgress(String taskId, MinionTaskProgressStats progress)
deleteTaskProgress(String taskId)
Configuration
In order to plug the desired implementation of
MinionTaskProgressManager
there is a new config introducedIf the config is not provided or minion is unable to load the instance of that class, the default implementation
DefaultMinionTaskProgressManager
will be used.This is the same logic of how the existing progress is being maintained in minion memory.
Track more stats with
MinionTaskProgressStats
The info maintained by MinionEventObserver in current implementation is not enough to get the granular view on a task's progress. A new wrapper class
MinionTaskProgressStats
is introduced to track and manage more stats.Stats tracked right now
MinionTaskState
)New stats added
New interface methods on
MinionEventObserver
Few new interface methods are added to
MinionEventObserver
along with the above improvementsMinionTaskProgressStats getProgressStats()
: to get all the stats for the task being trackedcleanup()
: cleanup code which will be called when the observer is purged upon reaching the ttl thresholdOnboard MergeRollupTask
To try out the improvements, the MergeRollupTaskExecutor flow has been updated to enable tracking all the new stats. As the flow spans over
SegmentProcessorFramework
andSegmentMapper
as well the tracking has been updated there as well