appform-io / dropwizard-service-discovery

Zookeeper service discovery bundle and client for dropwizard.
Apache License 2.0
17 stars 30 forks source link

Added a PortSchemeResolver to fetch the schemeType from serverConfiguration #49

Closed koushikr closed 1 year ago

koushikr commented 1 year ago

Considered adding a ServerLifeCycleListener, and using the serverStatus method to derive the portScheme using which serviceProvider shall be built. The issue was, by this time, the lifecycle listener would have been invoked, (loadbalancers like traefik etc could have registered this),and on a failure of a serviceProvider creation, server would stop abruptly - may load to 502s from calls coming from load balancers and proxies during the sigterm.

Given it is a dropwizard app, relied on ServerFactory instead. The default resolver is written to get the port, which will suit most dw applications. The default factory that dropwizard creates itself is a DefaultConnectionFactory, and using the first applicationConnector to do the portSchemeMapping. - If you have multiple, you'll anyway need to create multiple sd-bundles, since serviceProvider accepts only one scheme anyway. It was the same I would have done for the resolver too.

Added tests for the same. @santanusinha : Please let me know if you want to fallback onto the ServerLifeCycleListener (if the small window of failures are actually not all that jarring, TBF - this implementation is to avoid them if we can). Both of them work of user configs only (one before starting and one after). So I thought it sane to rely on serverFactory instead to fail fast on serviceProvider creation errors if any. (There is a risk here too, if the serviceProvider is created but the rest of the code fails, then ranger 502s, but that can be solved by initial stalenessThreshold or marking the node healthy only after server starting).

Let me know which way you want to proceed. Either can be done.

[UPDATE]

Also @santanusinha can you pls once run a format code, according to your style guide pls, I use the style guide, https://github.com/google/styleguide/blob/gh-pages/intellij-java-google-style.xml - And I have tried to adjust to your style guide, seems like there may be some misses, would be helpful if you could run a style guide once.

[UPDATE]

[UPDATE]

koushikr commented 1 year ago

Have formatted it according to the styleguide shared. Also checked-in the style-guide for future developers.