Skip to content

Commit

Permalink
Call size method on Application to avoid copying instance info
Browse files Browse the repository at this point in the history
DiscoveryClient called getInstancesAsIsFromEureka in a couple locations to get
the number of instances for an application, when Application already has a method
for this purpose that doesn't also incur the overhead of defensively copying.

Additionally, fix a concurrency bug in Application.size, as it was
accessing the underlying instances field without synchronization.
  • Loading branch information
kilink committed Aug 17, 2023
1 parent 40a91d5 commit 418f1b1
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1115,7 +1115,7 @@ private void logTotalInstances() {
if (logger.isDebugEnabled()) {
int totInstances = 0;
for (Application application : getApplications().getRegisteredApplications()) {
totInstances += application.getInstancesAsIsFromEureka().size();
totInstances += application.size();
}
logger.debug("The total number of all instances in the client now is {}", totInstances);
}
Expand Down Expand Up @@ -1219,7 +1219,7 @@ private void updateDelta(Applications delta) {
* We find all instance list from application(The status of instance status is not only the status is UP but also other status)
* if instance list is empty, we remove the application.
*/
if (existingApp.getInstancesAsIsFromEureka().isEmpty()) {
if (existingApp.size() == 0) {
applications.removeApplication(existingApp);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ public void setName(String name) {
* @return the number of instances in this application
*/
public int size() {
return instances.size();
synchronized (instances) {
return instances.size();
}
}

/**
Expand Down Expand Up @@ -252,6 +254,7 @@ private void _shuffleAndStoreInstances(boolean filterUpInstances, boolean indexB
this.shuffledInstances.set(instanceInfoList);
}


private void removeInstance(InstanceInfo i, boolean markAsDirty) {
instancesMap.remove(i.getId());
synchronized (instances) {
Expand Down

0 comments on commit 418f1b1

Please sign in to comment.