Issue with 0.11 router - it doesn't see brokers

[leventov]

After updating Router from 0.10.1 to 0.11 it stops to see brokers.

Might be related to recent changes in TieredBrokerHostSelector, e. g. 74538c3#diff-09401bb9b0565d15079a30fd8683695e

cc @clintropolis

FYI @himanshug

[himanshug]

@leventov what version of code are brokers running ?

[leventov]

@himanshug 0.11.0. And 0.10.1 router sees them

[himanshug]

ok, that is unexpected. ... I think I tested it but I'll check it again and see if I'm able to repro it.

but can you check
http://coord-host:port/druid/coordinator/v1/cluster and http://router-host:port/druid/router/v1/brokers to see if brokers are showing up anywhere.

if not, then can you see in broker log if they printed https://github.com/druid-io/druid/blob/0.11.0/server/src/main/java/io/druid/curator/discovery/CuratorDruidNodeAnnouncer.java#L71

[himanshug]

updated http://router-host:port/druid/router/v1/brokers in above comment

[leventov]

@himanshug brokers are visible, for sure. It's from router 0.11 logs:

INFO [NodeTypeWatcher[broker]] i.d.c.d.CuratorDruidNodeDiscoveryProvider$NodeTypeWatcher - Received event [CHILD_ADDED] for Node[broker:DiscoveryDruidNode{druidNode=Druid
Node{serviceName='druid/metrics/bard', host='someHost', port=-1, plaintextPort=somePort, enablePlaintextPort=true, tlsPort=-1, enableTlsPort=false}, nodeType='broker', services={lookupNodeServi
ce=LookupNodeService{lookupTier='historical'}}}]. - []

but later:

ERROR [id] i.d.s.r.QueryHostFinder - WTF?! No server found for serviceName[druid:metrics:bard]. Using backup - []

Could it be druid/metrics/bard vs. druid:metrics:bard problem somehow?

[himanshug]

it looks like druid.service=druid/metrics/bard on brokers. and, on router config, you have druid:metrics:bard . From the code, it looks like there is magic replacement of '/' to ':' while announcement and discovery in the older code.
I think it would work fine if in the router config you used same service name as provider to broker in its properties i.e. druid/metrics/bard. please see if that works.

if that is indeed the problem, then we'll probably mention it in release notes. this magic is unnecessary and confusing. it was probably needed in older code because serviceName was used in zk path.

[drcrallen]

If that is the case, its a pretty big breaking change

[himanshug]

yes, it would be incompatible but still allows rolling upgrade. there wouldn't be much value in keeping the magic happening. To remove it, at some point we would need to be incompatible. This is a major release, so we can hopefully afford that.

[gianm]

Would druid/metrics/bard work in the router config in older versions (0.10) too? If so then I would say it makes sense to note that you may have transformed / to : in your configs before, but it's not necessary, and will not continue to work. So you should switch them to / which will work with both the old and new versions.

[himanshug]

Yes, druid/metricss/bard would work in router config in older versions ( 0.10.x ) too. old external-discovery code does a translation from '/' to ':' on discoverer side ( https://github.com/druid-io/druid/blob/master/server/src/main/java/io/druid/curator/discovery/ServerDiscoveryFactory.java#L52 ) via ( https://github.com/druid-io/druid/blob/master/server/src/main/java/io/druid/curator/discovery/CuratorServiceUtils.java#L41 )

so technically, serviceName could have '/' in it earlier but it was always translated to ':' on both sides.

[himanshug]

@leventov did druid/metrics/bard change in router config work ?

[leventov]

Not yet 100% sure but at the moment it seems that it works. Also it seems that druid.router.coordinatorServiceName needs to be changed. Despite release notes, it seems to me it is so easy to forget this that I think we need to convert colons to slashes in Druid

[himanshug]

I thought about it a bit more and things wouldn't work if we put an auto replacement of ':' to '/' .

Let us you have two brokers with service names "druid:broker-tier1" and "druid/broker-tier2". and on router side you refer to them by "druid:broker-tier1" and "druid:broker-tier2".

If auto replacement from ':' to '/' is done on discovery side(i.e. at router) only then new router wouldn't be able to see first broker.
And if we did auto replacement on announcement side as well (i.e. at broker) then both brokers would announce "druid/broker-tier1" and "druid/broker-tier2" and old router code wouldn't be able to find them.

Pretty much all releases have a "Upgrading from release X-1" and contain information about actions need to be take by users before upgrade. This would be documented there and may be in bold to emphasize.

I understand that config change would happen for all service discovery properties such as druid.router.coordinatorServiceName too in other node configs as well.