-
Notifications
You must be signed in to change notification settings - Fork 823
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
[#4485] Adjusting the address isolation logic of the servicecomb engine registration and config center #4486
Conversation
…b engine registration and configuration center
// if address in same zone will be true; others will be false. | ||
private final Map<String, Boolean> addressCategory = new HashMap<>(); | ||
private final Set<String> addressCategory = new HashSet<>(); |
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.
code comments not correct.
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.
fixed
private List<String> addresses = new ArrayList<>(); | ||
private volatile List<String> addresses = new ArrayList<>(); | ||
|
||
private final List<String> default_isolation_address = new ArrayList<>(); |
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.
Naming convention
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.
fixed
this.defaultAddress.addAll(addresses); | ||
this.index = addresses.size() > 0 ? random.nextInt(addresses.size()) : 0; | ||
this.addressCategory.addAll(addresses); | ||
this.index = !addresses.isEmpty() ? random.nextInt(addresses.size()) : 0; |
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.
addresses and addressCategory may have different size and cause IndexOutOfBounds
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.
fixed
if (!addresses.isEmpty()) { | ||
return getCurrentAddress(addresses); | ||
} | ||
return getInitAddress(); | ||
LOGGER.warn("all address is isolation, please check server status."); |
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.
All addresses are isolated.
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.
fixed
} | ||
LOGGER.warn("all auto discovery address is isolation, please check server status."); |
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.
Refer to above
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.
fixed
No description provided.