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

Eureka Server and Client incompatible between Angel (SR6) and Brixton (SR1) when using custom eureka.instance.metadataMap.instanceId #1111

Closed
copa2 opened this issue Jun 16, 2016 · 10 comments
Labels
Milestone

Comments

@copa2
Copy link

copa2 commented Jun 16, 2016

As requested in #978, opened this new ticket.

When Angel client uses custom 'eureka.instance.metadataMap.instanceId' the
EurekaServer (Brixton.SR1) is not compatible with a Eureka Client (Angel.SR6).

In my case the Angel client has:

eureka:  
  instance:
    metadataMap:
      instanceId: ${spring.application.name}:${spring.application.instance_id:${random.value}}

In this case the client tries to register itself all 30 sec:

2016-06-16 17:37:39.620  INFO 11044 --- [pool-2-thread-1] com.netflix.discovery.DiscoveryClient    : DiscoveryClient_XXX-EUREKA-CLIENT-ANGEL/U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6 - Re-registering apps/XXX-EUREKA-CLIENT-ANGEL
2016-06-16 17:37:39.620  INFO 11044 --- [pool-2-thread-1] com.netflix.discovery.DiscoveryClient    : DiscoveryClient_XXX-EUREKA-CLIENT-ANGEL/U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6: registering service...
2016-06-16 17:37:39.621  INFO 11044 --- [pool-2-thread-1] com.netflix.discovery.DiscoveryClient    : DiscoveryClient_XXX-EUREKA-CLIENT-ANGEL/U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6 - registration status: 204
2016-06-16 17:38:09.661  INFO 11044 --- [pool-2-thread-1] com.netflix.discovery.DiscoveryClient    : DiscoveryClient_XXX-EUREKA-CLIENT-ANGEL/U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6 - Re-registering apps/XXX-EUREKA-CLIENT-ANGEL
2016-06-16 17:38:09.661  INFO 11044 --- [pool-2-thread-1] com.netflix.discovery.DiscoveryClient    : DiscoveryClient_XXX-EUREKA-CLIENT-ANGEL/U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6: registering service...
2016-06-16 17:38:09.662  INFO 11044 --- [pool-2-thread-1] com.netflix.discovery.DiscoveryClient    : DiscoveryClient_XXX-EUREKA-CLIENT-ANGEL/U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6 - registration status: 204

Brixton.SR1 Eureka Server logs:

2016-06-16 17:37:39.620  WARN 11392 --- [nio-8761-exec-5] c.n.e.registry.AbstractInstanceRegistry  : DS: Registry: lease doesn't exist, registering resource: XXX-EUREKA-CLIENT-ANGEL - U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6
2016-06-16 17:37:39.620  WARN 11392 --- [nio-8761-exec-5] c.n.eureka.resources.InstanceResource    : Not Found (Renew): XXX-EUREKA-CLIENT-ANGEL - U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6
2016-06-16 17:37:39.621  INFO 11392 --- [nio-8761-exec-7] c.n.e.registry.AbstractInstanceRegistry  : Registered instance XXX-EUREKA-CLIENT-ANGEL/U245496 with status UP (replication=false)
2016-06-16 17:38:09.661  WARN 11392 --- [nio-8761-exec-6] c.n.e.registry.AbstractInstanceRegistry  : DS: Registry: lease doesn't exist, registering resource: XXX-EUREKA-CLIENT-ANGEL - U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6
2016-06-16 17:38:09.661  WARN 11392 --- [nio-8761-exec-6] c.n.eureka.resources.InstanceResource    : Not Found (Renew): XXX-EUREKA-CLIENT-ANGEL - U245496:xxx-eureka-client-angel:983d24591515138d72191e85eb0479a6
2016-06-16 17:38:09.662  INFO 11392 --- [nio-8761-exec-9] c.n.e.registry.AbstractInstanceRegistry  : Registered instance XXX-EUREKA-CLIENT-ANGEL/U245496 with status UP (replication=false)

com.netflix.eureka.registry.AbstractInstanceRegistry.register will register with hostname(instanceInfo.getId()) when no instanceId is set(never set in Angel clients).

@copa2
Copy link
Author

copa2 commented Jun 16, 2016

Added to org.springframework.cloud.netflix.eureka.server.CloudJacksonJson.CloudInstanceInfoDeserializer

public InstanceInfo deserialize(JsonParser jp, DeserializationContext context) throws IOException {
            InstanceInfo instanceInfo = super.deserialize(jp, context);
            String instanceId = instanceInfo.getMetadata().get("instanceId");
            if(instanceInfo.getInstanceId() == null && instanceId != null) {
                String hostName = instanceInfo.getHostName();
                if (StringUtils.hasText(hostName) && !instanceId.startsWith(hostName)) {
                    instanceId = hostName + ":" + instanceId;
                }
                Field field = ReflectionUtils.findField(InstanceInfo.class, "instanceId");
                ReflectionUtils.makeAccessible(field);
                ReflectionUtils.setField(field, instanceInfo, instanceId);
            }
            return instanceInfo;
        }

but this deserializer is not used in the com.netflix.eureka.registry.AbstractInstanceRegistry.register. It still uses com.netflix.discovery.converters.EurekaJacksonCodec.InstanceInfoDeserializer. The reason is that dezerializer gets initialized at startup incom.netflix.discovery.provider.DiscoveryJerseyProvider.DiscoveryJerseyProvider(EncoderWrapper, DecoderWrapper) where it explicitly gets LegacyJacksonJson deserializer.

A ugly but working hack is to start EurekaServer with preinitialized codec:

public static void main(String[] args) {
        // Dirty hack, which injects CloudJacksonJson as encoder/decoder in com.netflix.discovery.provider.DiscoveryJerseyProvider
        CodecWrappers.registerWrapper(new CloudJacksonJson() {
            @Override
            public String codecName() {
                return getCodecName(LegacyJacksonJson.class);
            }
         // NOTE: This CloudJacksonJson contains the above extension with custom deserialize
        }); 
        new SpringApplicationBuilder(Application.class).web(true).run(args);
    }

There must be a cleaner way. I have to test this a little more.

Added example projects with this hack to: https://github.com/copa2/brixtonangelcompatibility

@spencergibb
Copy link
Member

@dsyer (and @copa2) if you want to take a look at the brixton-server-angel-compat branch. It's close. There are 2 404/reregistrations, then it settles down. @copa2 does yours do that?

@spencergibb
Copy link
Member

@copa2 ok, with just your hack I get the 2 404's and then things settle down fine. It registers with just hostname first (I saw it in the eureka console), then with the instanceid.

@william-tran
Copy link

Does CloudInstanceInfoDeserializeralso need to map $.instance.metadata.instanceId to InstanceInfo.instanceId, similar to what the serializer does?

https://github.com/spring-cloud/spring-cloud-netflix/blob/master/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/CloudJacksonJson.java#L144

Here's what an Angel client (SCS) POSTs to /eureka/apps/{appId} for reference.

{
    "instance": {
        "hostName": "hello-service-angel.coral.springapps.io",
        "app": "HELLO-SERVICE-ANGEL",
        "ipAddr": "10.68.196.142",
        "status": "UP",
        "overriddenstatus": "UNKNOWN",
        "port": {
            "@enabled": "true",
            "$": "80"
        },
        "securePort": {
            "@enabled": "true",
            "$": "443"
        },
        "countryId": 1,
        "dataCenterInfo": {
            "@class": "com.netflix.appinfo.InstanceInfo$DefaultDataCenterInfo",
            "name": "MyOwn"
        },
        "leaseInfo": {
            "renewalIntervalInSecs": 30,
            "durationInSecs": 90,
            "registrationTimestamp": 0,
            "lastRenewalTimestamp": 0,
            "evictionTimestamp": 0,
            "serviceUpTimestamp": 0
        },
        "metadata": {
            "instanceId": "25986c17-5485-4162-7e78-1d2d8fee95ab"
        },
        "homePageUrl": "http://hello-service-angel.coral.springapps.io:80/",
        "statusPageUrl": "http://hello-service-angel.coral.springapps.io:80/info",
        "healthCheckUrl": "http://hello-service-angel.coral.springapps.io:80/health",
        "secureHealthCheckUrl": "https://hello-service-angel.coral.springapps.io:443/health",
        "vipAddress": "hello-service-angel",
        "secureVipAddress": "hello-service-angel",
        "isCoordinatingDiscoveryServer": false,
        "lastUpdatedTimestamp": 1466125721975,
        "lastDirtyTimestamp": 1466125722710
    }
}

@copa2
Copy link
Author

copa2 commented Jun 17, 2016

@spencergibb Yes we seem to apply the same fixes. Yours cleaner and at the right places :-)
Here a gist for showing the communication pattern:
https://gist.github.com/copa2/68f357573975f98bd1177d876ead05e6
Regarding the re-registration. Its the same pattern also happening without metadataMap.instanceId.
@william-tran: Yes thats what the fixes do.

@spencergibb
Copy link
Member

@copa2 I think we may be seeing multiple threads on the client happen out of order, the first 404 is a renew that happens before register.

I had a hunch and ran the Angel Eureka Client against Angel Eureka server and see the same 2 404's. I think it's an issue with the angel client and sending the heartbeat's before registration. I think my fix in https://github.com/spring-cloud/spring-cloud-netflix/tree/brixton-server-angel-compat is as good as it gets.

@william-tran my branch linked above adds the functionality to the deserializer.

@copa2
Copy link
Author

copa2 commented Jun 17, 2016

I see it like you. I think your fix in brixton-server-angel-compat should be applied.

Also looked a little bit more into registration of Angel Client. It starts 2 threads which do registration (HeartbeatThread and InstanceInfoReplicator). First renew happens in the HeartbeatThread before registration. So its normal to get the first 404. The second 404 is because the in Brixton Server validates lastDirtyTimestamp with the one registered.

@spencergibb spencergibb added this to the 1.1.3 milestone Jun 17, 2016
spencergibb added a commit that referenced this issue Jun 17, 2016
Updates so CloudJacksonJson extension is properly recognized by Eureka
Server as LegacyJacksonJson.

see gh-978
fixes gh-1111
@spencergibb
Copy link
Member

Closed via 5d6590d

spencergibb added a commit that referenced this issue Jun 17, 2016
Updates so CloudJacksonJson extension is properly recognized by Eureka
Server as LegacyJacksonJson.

see gh-978
fixes gh-1111
@mtritschler
Copy link

@spencergibb, is there an ETA for a release of this fix in spring-cloud-netflix:1.1.3 and/or Brixton.SR2?

@spencergibb
Copy link
Member

@mtritschler no, but soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants