[Zen2] Add UnicastConfiguredHostsResolver #32642
Conversation
The `PeerFinder`, introduced in elastic#32246, obtains the collection of seed addresses configured by the user from a `ConfiguredHostsResolver`. In reality this collection comes from the `UnicastHostsProvider` via a slightly complicated threading model that performs the resolution of hostnames to addresses using a dedicated `ExecutorService`. This commit introduces an adapter to allow the `PeerFinder` to obtain its seed addresses in this manner.
DaveCTurner
added
>enhancement
v7.0.0
:Distributed Coordination/Cluster Coordination
|
Pinging @elastic/es-distributed |
|
|
||
| @Override | ||
| public void onAfter() { | ||
| super.onAfter(); |
There was a problem hiding this comment.
this does nothing. Not necessary
| transportService.getThreadPool().generic().execute(new AbstractRunnable() { | ||
| @Override | ||
| public void onFailure(Exception e) { | ||
| } |
There was a problem hiding this comment.
let's log the exception here.
| } | ||
|
|
||
| public void resolveConfiguredHosts(Consumer<List<TransportAddress>> consumer) { | ||
| assert lifecycle.started() : lifecycle; |
There was a problem hiding this comment.
I wonder if this might be called while the service is being stopped.
Note that we might also make use of AbstractLifecycleRunnable here (just found that class by chance)
There was a problem hiding this comment.
I changed the assert to a guard in a8a6393 but won't use AbstractLifecycleRunnable since it doesn't run onAfterInLifecycle method if the lifecycle state is STOPPED, which means this might break if stopped and then started again.
There was a problem hiding this comment.
However I also added a check for the lifecycle state in the action itself in c06a786
| @Before | ||
| public void startResolver() { | ||
| final Settings settings = Settings.builder().put(NODE_NAME_SETTING.getKey(), "node").build(); | ||
| threadPool = new ThreadPool(settings); |
| private final Supplier<ExecutorService> executorServiceFactory; | ||
|
|
||
| public UnicastConfiguredHostsResolver(Settings settings, TransportService transportService, UnicastHostsProvider hostsProvider, | ||
| Supplier<ExecutorService> executorServiceFactory) { |
There was a problem hiding this comment.
why not create the executorservice in the doStart method of this class? AFAICS, this will be the only user of this factory, no?
There was a problem hiding this comment.
Yes, that's nicer. I chose appropriate-looking names for things based on those in UnicastZenPing in 4c3060a
| protected void doClose() { | ||
| } | ||
|
|
||
| public void resolveConfiguredHosts(Consumer<List<TransportAddress>> consumer) { |
|
|
||
| unicastConfiguredHostsResolver.resolveConfiguredHosts(resolvedAddresses -> { | ||
| try { | ||
| assertTrue(startLatch.await(1, TimeUnit.SECONDS)); |
|
|
||
| assertThat(resolvedAddressesRef.get(), nullValue()); | ||
| startLatch.countDown(); | ||
| assertTrue(endLatch.await(1, TimeUnit.SECONDS)); |
|
|
||
| import static org.elasticsearch.discovery.zen.UnicastZenPing.DISCOVERY_ZEN_PING_UNICAST_CONCURRENT_CONNECTS_SETTING; | ||
|
|
||
| public class UnicastConfiguredHostsResolver extends AbstractLifecycleComponent implements ConfiguredHostsResolver { |
There was a problem hiding this comment.
I wonder if we should call this PeerFinderHostsResolver. This class, with its quite unusual interface (i.e. calling the callback only sometimes) is very much tailored towards use in PeerFinder whereas the name here suggests it is more than that.
There was a problem hiding this comment.
We discussed this and will leave it as-is for now, but will contemplate folding some of the interfaces with a single production implementation into methods on PeerFinder at a later date.
4 participants
The
PeerFinder, introduced in #32246, obtains the collection of seedaddresses configured by the user from a
ConfiguredHostsResolver. In realitythis collection comes from the
UnicastHostsProvidervia a slightlycomplicated threading model that performs the resolution of hostnames to
addresses using a dedicated
ExecutorService. This commit introduces anadapter to allow the
PeerFinderto obtain its seed addresses in this manner.