Skip to content
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

Merged
merged 4 commits into from
Sep 2, 2024

Conversation

chengyouling
Copy link
Contributor

No description provided.

…b engine registration and configuration center
@chengyouling chengyouling self-assigned this Aug 31, 2024
Comment on lines 66 to 67
// 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<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code comments not correct.

Copy link
Contributor Author

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<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming convention

Copy link
Contributor Author

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;
Copy link
Contributor

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

Copy link
Contributor Author

@chengyouling chengyouling Sep 2, 2024

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.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All addresses are isolated.

Copy link
Contributor Author

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.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refer to above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@liubao68 liubao68 merged commit 6406335 into apache:2.8.x Sep 2, 2024
6 checks passed
@chengyouling chengyouling deleted the 2.8.x-address branch September 19, 2024 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants