add dubbo-samples-consul demo

[Moriadry-zz]

add dubbo-samples-consul demo

[beiwei30]

@moriadry I added a very basic consul demo: https://github.com/apache/incubator-dubbo-samples/tree/master/dubbo-samples-consul. Feel free to enhance.

With this sample, the TCP check works as expected, pls. take a look.

[Moriadry-zz]

@beiwei30 yes, I will take a look, thanks.

[beiwei30]

the only flaw of TCP check is: if there's no client connected, the provider will keep printing when consul send a TCP request to dubbo's port, which is quite annoying:

Apr 30, 2019 2:12:38 PM org.apache.dubbo.remoting.transport.AbstractServer warn
WARNING:  [DUBBO] All clients has disconnected from 0.0.0.0/0.0.0.0:20880. You can graceful shutdown now., dubbo version: 2.7.1, current host: 30.5.124.198
Apr 30, 2019 2:12:48 PM org.apache.dubbo.remoting.transport.AbstractServer warn
WARNING:  [DUBBO] All clients has disconnected from 0.0.0.0/0.0.0.0:20880. You can graceful shutdown now., dubbo version: 2.7.1, current host: 30.5.124.198

[Moriadry-zz]

Hi Ian, it seems the sample works fine. I believe some code in ConsulRegistry.java went wrong, they don't work correctly with the TCP check. you can get correct response because you use the subscribed variable to get URL subscribed, rather than the msg from consul(not quite sure about this).

pls check this branch : https://github.com/Moriadry/incubator-dubbo/tree/feature/fordebug with this two different version of ConsulRegistry, and debug ConsulRegistryTest.testSubscribe locally, you will find the difference.

you can debug ConsulRegistryTest.testSubscribe.java, make a breakpoint at https://github.com/apache/incubator-dubbo/blob/e2f3346d9d1dff2bf6bf3ac2802e83d46b9923c0/dubbo-registry/dubbo-registry-consul/src/main/java/org/apache/dubbo/registry/consul/ConsulRegistry.java#L135
you will figure out we cannot get a NonEmpty HealthService list response returned. But with http-ttl Check, we can get correct HealthService list.

I believe our usage of TCP check is incorrect, we can spend more time on this to figure it out. I'm trying to fix it. I'm on my vocation those days, so it needs a few days.

[Moriadry-zz]

Or we can just use http ttl Check, those two checks are both the typical usage provided by consul.io, I believe their robustness can be consider mostly equal :) In case we can't figure this out :(

[beiwei30]

@moriadry pls. consider to create a http ttl check pull request, based on my previous change.

[beiwei30]

pls check this branch : https://github.com/Moriadry/incubator-dubbo/tree/feature/fordebug with this two different version of ConsulRegistry, and debug ConsulRegistryTest.testSubscribe locally, you will find the difference.

you can debug ConsulRegistryTest.testSubscribe.java, make a breakpoint at https://github.com/apache/incubator-dubbo/blob/e2f3346d9d1dff2bf6bf3ac2802e83d46b9923c0/dubbo-registry/dubbo-registry-consul/src/main/java/org/apache/dubbo/registry/consul/ConsulRegistry.java#L135
you will figure out we cannot get a NonEmpty HealthService list response returned. But with http-ttl Check, we can get correct HealthService list.

It this the exact change I have merged into the main trunk?

[beiwei30]

@moriadry I checked your unit test, since you do not register any service in advanced, isn't it expected an empty list returned?

[Moriadry-zz]

@beiwei30 With my uint test, if you use two different consulregistry(tcp and http), getHealthService will return different result(one is empty, one is not, when you register same service again). In your merged version, still use tcp, but lookup URL method without setPassing(true).

Let me just create a http-ttl-check pull request anyway.

[moonlighf]

how to solve the provider will keep printing when consul send a TCP request to dubbo's port and print [DUBBO] All clients has disconnected from 0.0.0.0/0.0.0.0:20880.