-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Expose MergeRateLimiter in runOnMergeFinished hook #15507
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
base: main
Are you sure you want to change the base?
Conversation
Changes runOnMergeFinished method signature in ConcurrentMergeScheduler to provide access to OneMerge and MergeRateLimiter parameters. This allows applications to monitor merge performance metrics like pause time, throttling behavior, and throughput. - Changed runOnMergeFinished from package-private to protected - Added OneMerge merge parameter for merge context - Added MergeRateLimiter rateLimiter parameter for metrics access This enables custom monitoring and observability for merge operations without requiring internal API access.
|
[Disclaimer: @zihanx and I work together, at Amazon customer facing product search] Thanks @zihanx -- these metrics are really helpful merge intensive indexing. Lucene applies backpressure (halts incoming indexing threads to give merge backlog a chance to get back under the hard merge count limit) and you are flying blind if there's no way to monitor it. Could you add |
mikemccand
left a comment
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.
Looks great -- thank you @zihanx!
Oh sorry I forgot this the first time around: how about a CHANGES.txt entry describing this, and how it enables visibility on this important Lucene behavior (backpressure to indexing when there is too much merge debt)?
Thanks @mikemccand! I have included an entry in |
mikemccand
left a comment
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.
Thanks @zihanx -- this looks great. I'll try to merge soon if there are no objections/feeddback...
Summary
This PR enhances the
runOnMergeFinishedhook inConcurrentMergeSchedulerto provide access to
OneMergeandMergeRateLimiterparameters.Motivation
Currently, applications that subclass
ConcurrentMergeSchedulerto monitormerge behavior have limited context in the
runOnMergeFinishedhook. TheMergeRateLimitercontains valuable metrics about merge performance(pause time, throttling, MB/sec) that would be useful for:
Changes
runOnMergeFinishedfrom package-private toprotectedto allow subclassingOneMerge mergeparameter for merge contextMergeRateLimiter rateLimiterparameter to expose merge rate metricsBackward Compatibility
This is a breaking change for any code that currently overrides
runOnMergeFinished,but since the method was package-private, external impact should be minimal.
Example Use Case
Applications can now override this method to emit custom metrics: