[fix][broker] ExtensibleLoadManager: handle SessionReestablished and Reconnected events to re-register broker metadata - #24932
Conversation
…sionRebuild (apache#24907) Signed-off-by: Dream95 <zhou_8621@163.com>
lhotari
left a comment
There was a problem hiding this comment.
LGTM. Good catch @Dream95! This looks like a production code bug in Extensible LoadManager's BrokerRegistryImpl since it didn't register to handle the reconnection event. For that reason, I think it's worth changing the title of the PR to describe this.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #24932 +/- ##
============================================
- Coverage 74.34% 74.24% -0.11%
- Complexity 33532 33880 +348
============================================
Files 1913 1913
Lines 149419 149427 +8
Branches 17356 17358 +2
============================================
- Hits 111089 110941 -148
- Misses 29480 29633 +153
- Partials 8850 8853 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Great catch! Can we backport this to 3.3, 4.0, and 4.1? |
|
note: when the broker registry migrated to the metadata cache from the lock manager, this session handler logic was not captured. ref: #23298 |
|
Not needed for branch-3.0 since #23298 isn't applied to that branch. |
…Reconnected events to re-register broker metadata (apache#24932) Signed-off-by: Dream95 <zhou_8621@163.com> (cherry picked from commit 95c1dab) (cherry picked from commit 4df635a)
…Reconnected events to re-register broker metadata (apache#24932) Signed-off-by: Dream95 <zhou_8621@163.com> (cherry picked from commit 95c1dab) (cherry picked from commit 4df635a)
Fixes #24907
Motivation
The test code has an incorrect variable name,and re-registration will not happen until the ExtensibleLoadManagerImpl.monitorTask execute.
Modifications
Add re-register when session reestablished.
Verifying this change
This change is already covered by existing tests.
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: Dream95#2