[ISSUE #7087] fix:consumer offset compatible problem caused by LmqConsumerOffsetManager #7090
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #7090 +/- ##
=============================================
- Coverage 42.78% 42.70% -0.08%
+ Complexity 9304 9288 -16
=============================================
Files 1138 1138
Lines 81252 81309 +57
Branches 10636 10643 +7
=============================================
- Hits 34762 34726 -36
- Misses 42152 42241 +89
- Partials 4338 4342 +4
... and 24 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
RongtongJin
changed the title
|
@humkum It seems that changes involving file names are generic, so consider placing the compatibility logic in the parent class ConfigManager Please help review whether such modify is appropriate. @RongtongJin @tianliuliu |
| @@ -36,6 +40,92 @@ public LmqConsumerOffsetManager(BrokerController brokerController) { | |||
| super(brokerController); | |||
| } | |||
|
|
|||
| @Override | |||
| public boolean load() { | |||
There was a problem hiding this comment.
IMO, compatibility in the ConfigManager would be better
There was a problem hiding this comment.
Compatibility logic can not remove to parent class, because it involves many different implementation methods, such as configFilePath, which can not obtain in parent class.
humkum
force-pushed
the
fix_lmq_compatible
branch
from
fde0f80
to
80f8fa3
Compare
|
I made some changes to fix the compatibility issues for upgrades and rollbacks, please help to review, thanks. @RongtongJin @tianliuliu |
…ager
Which Issue(s) This PR Fixes
Fixes #7087
Brief Description
How Did You Test This Change?