Service discovery with curator

[Marcin Cylke]

Hi

I'm working my way through Curator features. I'd like to implement a simple ServiceDiscovery service. 

For that scenerio I've implemented a class that advertises its services (https://github.com/zygm0nt/curator-playground/blob/master/src/main/java/pl/touk/curator/WorkerAdvertiser.java) and a class that tries to find a suitable service (https://github.com/zygm0nt/curator-playground/blob/master/src/main/java/pl/touk/curator/WorkerFinder.java)

The whole thing is run by this test:

https://github.com/zygm0nt/curator-playground/blob/master/src/main/java/pl/touk/curator/ServiceDiscoverer.java

The problem is that although the service advertises its presence and creates a node on ZK filesystem, as soon as close() is called here (https://github.com/zygm0nt/curator-playground/blob/master/src/main/java/pl/touk/curator/WorkerAdvertiser.java#L47) the znode disappears. That is why it can't be found by a discovering service (WorkerFinder).

What should be the correct approach to implement service discovery? Am I using close() method too early?

Regards

Marcin

[Jordan Zimmerman]

You should only call close when you are shutting down your application.

-Jordan

--
You received this message because you are subscribed to the Google Groups "curator-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to curator-user...@googlegroups.com.
To post to this group, send email to curato...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msg/curator-users/-/jhq-jqgQdcQJ.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Marcin Cylke]

On Monday, 25 February 2013 21:44:53 UTC+1, Jordan Zimmerman wrote:

You should only call close when you are shutting down your application.

This helped, thanks!

Regards

Marcin 

[Marcin Cylke]

Now the instance can be found, by I can't deserialize the already serialized instance. I've created a test, would you have any time to look into this?

The test is here:

https://github.com/zygm0nt/curator-playground/blob/master/src/test/java/pl/touk/curator/JsonSerializerTest.java 

The error I'm getting looks like this:

org.codehaus.jackson.map.exc.UnrecognizedPropertyException: Unrecognized field "address" (Class com.netflix.curator.x.discovery.ServiceInstance), not marked as ignorable

 at [Source: [B@5e6458a6; line: 1, column: 13] (through reference chain: com.netflix.curator.x.discovery.ServiceInstance["address"])

at org.codehaus.jackson.map.exc.UnrecognizedPropertyException.from(UnrecognizedPropertyException.java:53)

at org.codehaus.jackson.map.deser.StdDeserializationContext.unknownFieldException(StdDeserializationContext.java:246)

at org.codehaus.jackson.map.deser.StdDeserializer.reportUnknownProperty(StdDeserializer.java:604)

at org.codehaus.jackson.map.deser.StdDeserializer.handleUnknownProperty(StdDeserializer.java:590)

at org.codehaus.jackson.map.deser.BeanDeserializer.handleUnknownProperty(BeanDeserializer.java:689)

at org.codehaus.jackson.map.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:514)

at org.codehaus.jackson.map.deser.BeanDeserializer.deserialize(BeanDeserializer.java:350)

at org.codehaus.jackson.map.ObjectReader._bindAndClose(ObjectReader.java:477)

at org.codehaus.jackson.map.ObjectReader.readValue(ObjectReader.java:260)

at pl.touk.curator.JacksonInstanceSerializer.deserialize(JacksonInstanceSerializer.java:33)

at pl.touk.curator.JsonSerializerTest.shouldDeserializeFromString(JsonSerializerTest.java:25)

at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)

at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)

at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)

at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)

at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)

at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)

at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)

at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)

at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)

at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)

at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)

at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)

at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)

at org.junit.runners.ParentRunner.run(ParentRunner.java:300)

at org.junit.runner.JUnitCore.run(JUnitCore.java:157)

at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:63)

If that doesn't ring a bell, perhaps you could suggest some other option for serializing/deserializing service instances to be used with curator?

 

Regards

Marcin

[Jordan Zimmerman]

That looks like a Jackson problem. I can't help you with that ;)

-JZ

--
You received this message because you are subscribed to the Google Groups "curator-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to curator-user...@googlegroups.com.
To post to this group, send email to curato...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/curator-users/-/sOFqtgi8JCQJ.

[Marcin Cylke]

On Tuesday, 26 February 2013 19:22:31 UTC+1, Jordan Zimmerman wrote:

That looks like a Jackson problem. I can't help you with that ;)

Right! :) Fixed that by creating Jackson-compatible wrapper around ServiceInstance.

Regards

Marcin 

[Josh Rosenblum]

We encountered a similar issue using service discovery with an upgrade to (I believe) the 1.2 Curator branch. In our case -- and I actually think this is what is happening in the reported issue -- it appeared related to the choice to have ServiceInstance utilize a builder pattern. (Notice Jackson trying to use the BeanDeserializer in the stack trace above). While Jackson has promised better support for the builder pattern in 2.0, it's not yet GA. (In particular, ServiceInstance has no setters b/c it's designed with the builder pattern in mind; the default configurations of Jackson cannot, I believe, deserialize without setters on JsonInstanceSerializer line 50 -- I wasn't clear whether there was a particular configuration of Jackson that was assumed.) It would be interesting to see if the Jackson 2.0 GA can make this problem easier to solve in a more generic way.

We actually went a slightly different route than creating wrappers and utilized some of Jackson's configuration support as well as Curator service discovery's pluggable InstanceSerializer option. This avoids having to create a separate wrapper. It seemed like a way to get around Jackson complaining that it could not deserialize ServiceInstances (b/c they don't have setters).

We created a JacksonInstanceSerializer<T> that implements InstanceSerializer<T>. In its constructor, it takes a Jackson ObjectMapper, configures it to ignore unknown properties (mapper.configure(DeserializationConfig.Feature.FAIL_ON_UNKNOWN_PROPERTIES, false);), and then handles deserialization itself by pulling out the specific fields expected by ServiceInstanceBuilder. We then configure our service discovery using this JacksonInstanceSerializer. We weren't clear whether there was an implicit Jackson configuration assumed by x-service-discovery that would allow out-of-the-box Jackson to correctly deserialize using ServiceInstanceBuilder (or using ServiceInstance directly without setters, i.e. not the BeanDeserializer) -- perhaps there's a different way.

At the very least, I'm curious how others have seen the standard JsonInstanceSerializer.deserialize behave given that Jackson will attempt to use its BeanDeserializer and there are no setters on ServiceInstance. Would be great to do this in the least invasive / most standard way.

[Marcin Cylke]

On Wednesday, 27 February 2013 15:04:58 UTC+1, Josh Rosenblum wrote:

We created a JacksonInstanceSerializer<T> that implements InstanceSerializer<T>. In its constructor, it takes a Jackson ObjectMapper, configures it to ignore unknown properties (mapper.configure(DeserializationConfig.Feature.FAIL_ON_UNKNOWN_PROPERTIES, false);), and then handles deserialization itself by pulling out the specific fields expected by ServiceInstanceBuilder. We then configure our service discovery using this JacksonInstanceSerializer. We weren't clear whether there was an implicit Jackson configuration assumed by x-service-discovery that would allow out-of-the-box Jackson to correctly deserialize using ServiceInstanceBuilder (or using ServiceInstance directly without setters, i.e. not the BeanDeserializer) -- perhaps there's a different way.

The lack of setters is certainly a problem here, but even without Jackson 2.0 we were still able to fix the issue with only small code addition. 

 

At the very least, I'm curious how others have seen the standard JsonInstanceSerializer.deserialize behave given that Jackson will attempt to use its BeanDeserializer and there are no setters on ServiceInstance. Would be great to do this in the least invasive / most standard way.

I've used @JsonCreator annotation. I've implemented a class like this one:

public class JsonServiceInstance<T> extends ServiceInstance<T> {

    @JsonCreator

    public JsonServiceInstance(@JsonProperty("name") String name,

                               @JsonProperty("id") String id,

                               @JsonProperty("address") String address,

                               @JsonProperty("port") Integer port,

                               @JsonProperty("sslPort") Integer sslPort,

                               @JsonProperty("payload") T payload,

                               @JsonProperty("registrationTimeUTC") long registrationTimeUTC,

                               @JsonProperty("serviceType") ServiceType serviceType,

                               @JsonProperty("uriSpec") UriSpec uriSpec) {

        super(name, id, address, port, sslPort, payload, registrationTimeUTC, serviceType, uriSpec);

    }

} 

It is sufficient. Of course I had to change all the parsing methods to handle JsonServiceInstance. If it is still unclear you can check my github repo. I've pushed the working version there.

Regards

Marcin

[Josh Rosenblum]

On Wednesday, February 27, 2013 9:15:07 AM UTC-5, Marcin Cylke wrote:

On Wednesday, 27 February 2013 15:04:58 UTC+1, Josh Rosenblum wrote:

We created a JacksonInstanceSerializer<T> that implements InstanceSerializer<T>. In its constructor, it takes a Jackson ObjectMapper, configures it to ignore unknown properties (mapper.configure(DeserializationConfig.Feature.FAIL_ON_UNKNOWN_PROPERTIES, false);), and then handles deserialization itself by pulling out the specific fields expected by ServiceInstanceBuilder. We then configure our service discovery using this JacksonInstanceSerializer. We weren't clear whether there was an implicit Jackson configuration assumed by x-service-discovery that would allow out-of-the-box Jackson to correctly deserialize using ServiceInstanceBuilder (or using ServiceInstance directly without setters, i.e. not the BeanDeserializer) -- perhaps there's a different way.

The lack of setters is certainly a problem here, but even without Jackson 2.0 we were still able to fix the issue with only small code addition. 

 

Sounds like you encountered a similar issue. So again, I'm curious whether any other folks have examples of ServiceDiscovery working "out of the box" given this apparent mismatch between the builder pattern assumed by ServiceInstance and Jackson's default configuration. Seems like otherwise this might be an opportunity for a modification to the core service discovery types rather than individual workarounds?
 

At the very least, I'm curious how others have seen the standard JsonInstanceSerializer.deserialize behave given that Jackson will attempt to use its BeanDeserializer and there are no setters on ServiceInstance. Would be great to do this in the least invasive / most standard way.

I've used @JsonCreator annotation. I've implemented a class like this one:

public class JsonServiceInstance<T> extends ServiceInstance<T> {

    @JsonCreator

    public JsonServiceInstance(@JsonProperty("name") String name,

                               @JsonProperty("id") String id,

                               @JsonProperty("address") String address,

                               @JsonProperty("port") Integer port,

                               @JsonProperty("sslPort") Integer sslPort,

                               @JsonProperty("payload") T payload,

                               @JsonProperty("registrationTimeUTC") long registrationTimeUTC,

                               @JsonProperty("serviceType") ServiceType serviceType,

                               @JsonProperty("uriSpec") UriSpec uriSpec) {

        super(name, id, address, port, sslPort, payload, registrationTimeUTC, serviceType, uriSpec);

    }

} 

It is sufficient. Of course I had to change all the parsing methods to handle JsonServiceInstance. If it is still unclear you can check my github repo. I've pushed the working version there.

Yes, that's another option -- as you mention, it does necessitate changing type references in a number of places. Seems like two different approaches that involve either creating the deserializer so it can handle the builder pattern on the existing builder-based ServiceInstance type or creating an extension type that doesn't leverage the builder pattern. Both seem of comparable complexity -- but it seems like there's an opportunity to remove the need for either of these workarounds.

Thanks.
 

Regards

Marcin

[Marcin Cylke]

On Wednesday, 27 February 2013 16:05:24 UTC+1, Josh Rosenblum wrote:

Yes, that's another option -- as you mention, it does necessitate changing type references in a number of places. Seems like two different approaches that involve either creating the deserializer so it can handle the builder pattern on the existing builder-based ServiceInstance type or creating an extension type that doesn't leverage the builder pattern. Both seem of comparable complexity -- but it seems like there's an opportunity to remove the need for either of these workarounds.

Just out of curiosity, could you post somewhere the code for deserializer? I'm just curious how to do that kind of thing (custom deserializer for builder pattern) with Jackson.

Regards

Marcin 

[Jordan Zimmerman]

Can someone (or together) come up with a better implementation for this and put in a pull request?

-Jordan

--
You received this message because you are subscribed to the Google Groups "curator-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to curator-user...@googlegroups.com.
To post to this group, send email to curato...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/curator-users/-/we5sIGJNfqQJ.

[Josh Rosenblum]

Done: https://github.com/Netflix/curator/pull/255

I realized after the fact that this request probably has some of our own coding conventions in it (pretty much pulled verbatim from existing work) -- happy to standardize on the existing Curator code conventions and generate another pull if desired.

[Jordan Zimmerman]

I'll reformat it - no worries.

To view this discussion on the web visit https://groups.google.com/d/msg/curator-users/-/N71404cIiicJ.