[improve][broker] Replaced checkBackloggedCursors with checkBackloggedCursor(single subscription check) upon subscription - #19343
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19343 +/- ##
=============================================
+ Coverage 32.42% 62.50% +30.08%
- Complexity 6347 25420 +19073
=============================================
Files 1644 1818 +174
Lines 123712 133112 +9400
Branches 13486 14644 +1158
=============================================
+ Hits 40109 83205 +43096
+ Misses 77694 42231 -35463
- Partials 5909 7676 +1767
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Question: |
merlimat
left a comment
There was a problem hiding this comment.
I think it would be better to call it only for the single subscription, instead of for all of them
|
Also, If we still want to call this operation, checkBackloggedCursors asynchronously but more frequently, we could run such async operations in a separate thread and run it more frequently, every x milliseconds(out side of updateStats), if there are any requests (with deduplication). // Example var dedupedOps void schedule(Op op) // dedup/add op to dedupedOps. void start(){ while(true) } |
1f41b31 to
70f1883
Compare
Updated the code to check the single subscription. |
70f1883 to
559194d
Compare
…dCursor(single subscription check) from subscribe
559194d to
3cb8c55
Compare
|
@heesung-sn Please change the PR title.
|
|
Oops, thanks. Updated the title. |
tisonkun
left a comment
There was a problem hiding this comment.
Thanks for your contribution @heesung-sn! Merging..
…dCursor(single subscription check) upon subscription (#19343)
…dCursor(single subscription check) upon subscription (#19343)
…dCursor(single subscription check) upon subscription (apache#19343) (cherry picked from commit 6b067a5)
Motivation
Related to #19341,
checkBackloggedCursors()iterates all subscriptions for each subscribe call.checkBackloggedCursors()in the subscription path is redundant ascheckBackloggedCursors()is called inupdateStats()everystatsUpdateFrequencyInSecs(by default 1min).Modifications
Replaced
checkBackloggedCursors()withcheckBackloggedCursor()insubscribe()call. Upon the subscription creation, this will only check thebackloggedCursorfrom the subscription to create instead of checking all the subscriptions.Verifying this change
This change can be verified as follows:
simulate many concurrent subscriptions
before this PR
after this PR
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: heesung-sohn#24