-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
[ISSUE #8058] Support for upgrading metadata in json to rocksdb (#8045) #8116
Conversation
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.
I checked the code, and it seems that upgrading from the 5.2.0 version with RocksDB enabled to this modified version is fully compatible, isn't it?
没问题 |
this.configRocksDBStorage = new ConfigRocksDBStorage(filePath); | ||
return this.configRocksDBStorage.start(); | ||
} | ||
public final boolean loadDataVersion() { |
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.
Why use public final for method here?
kvDataVersion = StringUtils.isNotBlank(currDataVersionString) ? JSON.parseObject(currDataVersionString, DataVersion.class) : new DataVersion(); | ||
return true; | ||
} catch (Exception e) { | ||
throw new RuntimeException(e); |
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.
How about print some information for the exception here?
return true; | ||
} | ||
|
||
public boolean loadSubscriptionGroupAndForbidden() { |
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.
If you want to do as a pipeline case here , use if-else may be a more readable way.
Which Issue(s) This PR Fixes
Fixes #8058
Brief Description
How Did You Test This Change?