Armeria: Use Curator Framework instead of vanilla ZooKeeper for armeria-zookeeper

[trustin]

.. because it simplifies our code a lot and frees us from maintaining ZooKeeper-handling code. i.e. less bugs and test failures.

[trustin]

Note about dependencies:

The following would be the only dependencies involved:

• curator-framework
• curator-client
• zookeeper
• slf4j-api

We can exclude the following transitive dependencies, due to sorry state of ZooKeeper and Curator POM:

• guava (build-time only dependency since it's shaded)
• log4j (test-time only dependency)
• slf4j-log4j12 (test-time only dependency)
• netty (optional dependency)
• audience-annotations (documentation-purpose only)
• jline (only used for command line tools?)

[dongjinleekr]

Hello. I am currently working on this issue. Before progress more, I would like to ask some questions.

As all of you know, apache curator provides its core package (curator-client) with its high-level module, curator-x-discovery, which serves the Service Discovery feature.

My question is: Should we adapt curator-x-discovery for armeria's service discovery feature? There are pros and cons for each of strategy.

• Use curator-client only: We can maintain the current API & implementations, but can not make use of safe, verified module.
• Use curator-x-discovery: We can make use of the verified module, but there is some potential to change the current API, to be fitted into the curator-x-discovery module.

The one thing that should be noted is that We have to support the other Service Discovery frameworks in the future (issue https://github.com/line/armeria/issues/194). That is, avoiding curator-x-discovery does not eliminate the potential to make some changes to current Service Discovery API. If we support other Service Discovery Service (for both of registration/resolving), defining a new API would be required at some point.

[dongjinleekr]

Please note: https://curator.apache.org/apidocs/org/apache/curator/x/discovery/UriSpec.html

[trustin]

curator-x-discovery looks great, but it doesn't look compatible with how we store the server list in ZooKeeper. How about implementing both - one that's backward-compatible with the current implementation and the other that uses curator-x-discovery? It's OK to break the API compatibility as long as the data layout is compatible. If you are not interested in implemeting two, then you can just implement one you prefer.

[dongjinleekr]

It's almost done. I expect that I can open the pull request in next week.

However, I have a question: In the current work, I implemented ServerListenerBuilder which creates ServerListener instance by composing Runnable instances. Is this okay?

The reason why I concluded that we need such a thing follows. As all of you know, using Curator Framework with Armeria means that we have to define some ServerListener instances that start/stops one or more Curator instances[^1]. After writing those kinds of code one by one, I concluded that this way would be better:

1. Provide a builder which enables the easy building of ServerListener by composing Runnable instances in the core package.[^2]
2. Provide a utility class that creates Runnable instances which start/stop given CuratorFramework instance in armeria-zookeeper, on top of 1.
3. Add additional helper methods for building similar Runnables on other curator-recipes instances to 2. (Optional)
4. Move Zookeeper-based service discovery implementation into CuratorFramework.

How do you think? If you agree with 1, it would be better to open a separate issue - then I will open a PR on it first.

[dongjinleekr]

+1. I also concluded that CuratorFramework is sufficient for our Service Discovery feature - so we don't need curator-x-discovery module.

[trustin]

However, I have a question: In the current work, I implemented ServerListenerBuilder which creates ServerListener instance by composing Runnable instances. Is this okay?

That sounds interesting enough for a separate issue. Could you create a new issue and propose your idea there? (Example code preferred.)

[dongjinleekr]

I opened #1003 with an implementation. Please take a look when you are free.