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

updateDelta() bug? In come case, reconcileHashCode inconsistencies may occur after updating #1528

Open
q605450469 opened this issue Jan 16, 2024 · 0 comments

Comments

@q605450469
Copy link

q605450469 commented Jan 16, 2024

sorry for my poor English.

environment:
eureka-server: 1.7.2
eureka-client: 2.0.1

I have request monitoring for eureka server,and I found that /eureka/apps/ has irregular traffic surges, like this:
image

Then I checked the source code and learned about the update mechanism. I found that the following code seemed to have a bug.

the code from com.netflix.discovery.DiscoveryClient#updateDelta

private void updateDelta(Applications delta) {
        int deltaCount = 0;
        for (Application app : delta.getRegisteredApplications()) {
            for (InstanceInfo instance : app.getInstances()) {
                Applications applications = getApplications();
                String instanceRegion = instanceRegionChecker.getInstanceRegion(instance);
                if (!instanceRegionChecker.isLocalRegion(instanceRegion)) {
                    Applications remoteApps = remoteRegionVsApps.get(instanceRegion);
                    if (null == remoteApps) {
                        remoteApps = new Applications();
                        remoteRegionVsApps.put(instanceRegion, remoteApps);
                    }
                    applications = remoteApps;
                }

                ++deltaCount;
                if (ActionType.ADDED.equals(instance.getActionType())) {
                    Application existingApp = applications.getRegisteredApplications(instance.getAppName());
                    if (existingApp == null) {
                        //⭐️⭐️⭐️attention here! if existingApp == null, it will set whole application from delta. but the application from delta may contain multiple instances. Maybe contain instances with status down.
                        applications.addApplication(app);
                    }
                    logger.debug("Added instance {} to the existing apps in region {}", instance.getId(), instanceRegion);
                    applications.getRegisteredApplications(instance.getAppName()).addInstance(instance);
                } else if (ActionType.MODIFIED.equals(instance.getActionType())) {
                    Application existingApp = applications.getRegisteredApplications(instance.getAppName());
                    if (existingApp == null) {
                        //⭐️⭐️⭐️same problem also arises here 
                        applications.addApplication(app);
                    }
                    logger.debug("Modified instance {} to the existing apps ", instance.getId());

                    applications.getRegisteredApplications(instance.getAppName()).addInstance(instance);

                } else if (ActionType.DELETED.equals(instance.getActionType())) {
                    Application existingApp = applications.getRegisteredApplications(instance.getAppName());
                    if (existingApp != null) {
                        logger.debug("Deleted instance {} to the existing apps ", instance.getId());
                        existingApp.removeInstance(instance);
                        /*
                         * 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()) {
                            applications.removeApplication(existingApp);
                        }
                    }
                }
            }
        }
        logger.debug("The total number of instances fetched by the delta processor : {}", deltaCount);

        getApplications().setVersion(delta.getVersion());
        getApplications().shuffleInstances(clientConfig.shouldFilterOnlyUpInstances());

        for (Applications applications : remoteRegionVsApps.values()) {
            applications.setVersion(delta.getVersion());
            applications.shuffleInstances(clientConfig.shouldFilterOnlyUpInstances());
        }
    }

and when calculate ReconcileHashCode

    public void populateInstanceCountMap(Map<String, AtomicInteger> instanceCountMap) {
        for (Application app : this.getRegisteredApplications()) {
            //⭐️⭐️⭐️,it use getInstancesAsIsFromEureka(), this function will get instance in real time.
            for (InstanceInfo info : app.getInstancesAsIsFromEureka()) {
                AtomicInteger instanceCount = instanceCountMap.computeIfAbsent(info.getStatus().name(),
                        k -> new AtomicInteger(0));
                instanceCount.incrementAndGet();
            }
        }
    }
    @JsonIgnore
    public List<InstanceInfo> getInstancesAsIsFromEureka() {
        synchronized (instances) {
           return new ArrayList<InstanceInfo>(this.instances);
        }
    }

so, if I have a service, Go register immediately after unregister, I might get a delta like this:

<applications>
    <versions__delta>21908</versions__delta>
    <apps__hashcode>UP_399_</apps__hashcode>
    <application>
        <name>service-a</name>
        <instance>
            <instanceId>xxxx1</instanceId>
            <hostName>x.x.x.1</hostName>
            <app>service-a</app>
            <ipAddr>x.x.x.1</ipAddr>
            <status>DOWN</status>
            <overriddenstatus>UNKNOWN</overriddenstatus>
            <port enabled="true">80</port>
            <securePort enabled="false">443</securePort>
            ...
            <actionType>DELETED</actionType>
        </instance>
        <instance>
            <instanceId>xxxx2</instanceId>
            <hostName>x.x.x.2</hostName>
            <app>service-a</app>
            <ipAddr>x.x.x.2</ipAddr>
            <status>UP</status>
            <overriddenstatus>UNKNOWN</overriddenstatus>
            <port enabled="true">80</port>
            <securePort enabled="false">443</securePort>
            ...
            <actionType>ADDED</actionType>
        </instance>
    </application>
</applications>

At this time, if this application had only one instance before, the instance whose execution status is down will delete the local application, but an instance whose execution status is up will re-add the down service to the application's instances. calculating ReconcileHashCode will be inconsistent with the one in delta.

Please help me, did I understand it wrong? thranks.

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

No branches or pull requests

1 participant