Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple unicast host providers #31509

Merged
merged 2 commits into from
Jun 22, 2018

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Jun 21, 2018

This PR introduces support for multiple host providers. This allows the settings based hosts resolver to be just be treated as any other UnicastHostsProvider. The PR also introduces the notion of a HostsResolver so that plugins such as FileBasedDiscovery do not need to create their own thread pool for resolving hosts, making it easier to add new plugins.

@ywelsch ywelsch added >enhancement :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure >breaking-java v7.0.0 v6.4.0 labels Jun 21, 2018
@ywelsch ywelsch requested a review from tlrx June 21, 2018 16:40
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I like it! I left some minor comments around documentation.


import static java.util.Collections.emptyList;

public class SettingsBasedHostsProvider extends AbstractComponent implements UnicastHostsProvider {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some documentation please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I've pushed 0d54fd2


if (DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING.exists(settings)) {
configuredHosts = DISCOVERY_ZEN_PING_UNICAST_HOSTS_SETTING.get(settings);
// we only limit to 1 addresses, makes no sense to ping 100 ports
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: addresses -> address

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 0d54fd2

List<TransportAddress> buildDynamicHosts();
List<TransportAddress> buildDynamicHosts(HostsResolver hostsResolver);

interface HostsResolver {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a bit of documentation please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I've pushed 0d54fd2

futures = executorService.invokeAll(callables, resolveTimeout.nanos(), TimeUnit.NANOSECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return Collections.emptyList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ywelsch ywelsch merged commit f22f91c into elastic:master Jun 22, 2018
@ywelsch
Copy link
Contributor Author

ywelsch commented Jun 22, 2018

Thanks @tlrx

ywelsch added a commit that referenced this pull request Jun 22, 2018
Introduces support for multiple host providers, which allows the settings based hosts resolver to be
treated just as any other UnicastHostsProvider. Also introduces the notion of a HostsResolver so
that plugins such as FileBasedDiscovery do not need to create their own thread pool for resolving
hosts, making it easier to add new similar kind of plugins.
dnhatn added a commit that referenced this pull request Jun 23, 2018
* master:
  Add get field mappings to High Level REST API Client (#31423)
  [DOCS] Updates Watcher examples for code testing (#31152)
  TEST: Add bwc recovery tests with synced-flush index
  [DOCS] Move sql to docs (#31474)
  [DOCS] Move monitoring to docs folder (#31477)
  Core: Combine doExecute methods in TransportAction (#31517)
  IndexShard should not return null stats (#31528)
  fix repository update with the same settings but different type (#31458)
  Fix Mockito trying to mock IOException that isn't thrown by method (#31433) (#31527)
  Node selector per client rather than per request (#31471)
  Core: Combine messageRecieved methods in TransportRequestHandler (#31519)
  Upgrade to Lucene 7.4.0. (#31529)
  [ML] Add ML filter update API (#31437)
  Allow multiple unicast host providers (#31509)
  Avoid deprecation warning when running the ML datafeed extractor. (#31463)
  REST high-level client: add simulate pipeline API (#31158)
  Get Mapping API to honour allow_no_indices and ignore_unavailable (#31507)
  [PkiRealm] Invalidate cache on role mappings change (#31510)
  [Security] Check auth scheme case insensitively (#31490)
  In NumberFieldType equals and hashCode, make sure that NumberType is taken into account. (#31514)
  [DOCS] Fix REST tests in SQL docs
  [DOCS] Add code snippet testing in more ML APIs (#31339)
  Core: Remove ThreadPool from base TransportAction (#31492)
  [DOCS] Remove fixed file from build.gradle
  Rename createNewTranslog to fileBasedRecovery (#31508)
  Test: Skip assertion on windows
  [DOCS] Creates field and document level security overview (#30937)
  [DOCS] Significantly improve SQL docs
  [DOCS] Move migration APIs to docs (#31473)
  Core: Convert TransportAction.execute uses to client calls (#31487)
  Return transport addresses from UnicastHostsProvider (#31426)
  Ensure local addresses aren't null (#31440)
  Remove unused generic type for client execute method (#31444)
  Introduce http and tcp server channels (#31446)
dnhatn added a commit that referenced this pull request Jun 23, 2018
* 6.x:
  Avoid sending duplicate remote failed shard requests (#31313)
  Add get field mappings to High Level REST API Client Relates to #27205
  [DOCS] Updates Watcher examples for code testing (#31152)
  [DOCS] Move monitoring to docs folder (#31477)
  [DOCS] Fixes SQL docs in nav
  [DOCS] Move sql to docs
  IndexShard should not return null stats - empty stats or AlreadyCloseException if it's closed is better
  Clarify that IP range data can be specified in CIDR notation. (#31374)
  Remove some cases in FieldTypeLookupTests that are no longer relevant. (#31381)
  In NumberFieldType equals and hashCode, make sure that NumberType is taken into account. (#31514)
  fix repository update with the same settings but different type
  Revert "AwaitsFix FullClusterRestartIT#testRecovery"
  Upgrade to Lucene 7.4.0. (#31529)
  Avoid deprecation warning when running the ML datafeed extractor. (#31463)
  Retry synced-flush in FullClusterRestartIT#testRecovery
  Allow multiple unicast host providers (#31509)
  [ML] Add ML filter update API (#31437)
  AwaitsFix FullClusterRestartIT#testRecovery
  Fix missing historyUUID in peer recovery when rolling upgrade 5.x to 6.3 (#31506)
  Remove QueryCachingPolicy#ALWAYS_CACHE (#31451)
  Rename createNewTranslog to fileBasedRecovery (#31508)
  [DOCS] Add code snippet testing in more ML APIs (#31339)
  [DOCS] Remove fixed file from build.gradle
  [DOCS] Creates field and document level security overview (#30937)
  Test: Skip assertion on windows
  [DOCS] Move migration APIs to docs (#31473)
  Add a known issue for upgrading from 5.x to 6.3.0 (#31501)
  Return transport addresses from UnicastHostsProvider (#31426)
  Add Delete Snapshot High Level REST API
  Reload secure settings for plugins (#31481)
  [DOCS] Fix JDBC Maven client group/artifact ID
jasontedor added a commit to hub-cap/elasticsearch that referenced this pull request Jun 25, 2018
* elastic/master: (92 commits)
  Reduce number of raw types warnings (elastic#31523)
  Migrate scripted metric aggregation scripts to ScriptContext design (elastic#30111)
  turn GetFieldMappingsResponse to ToXContentObject (elastic#31544)
  Close xcontent parsers (partial) (elastic#31513)
  Ingest Attachment: Upgrade Tika to 1.18 (elastic#31252)
  TEST: Correct the assertion arguments order (elastic#31540)
  Add get field mappings to High Level REST API Client (elastic#31423)
  [DOCS] Updates Watcher examples for code testing (elastic#31152)
  TEST: Add bwc recovery tests with synced-flush index
  [DOCS] Move sql to docs (elastic#31474)
  [DOCS] Move monitoring to docs folder (elastic#31477)
  Core: Combine doExecute methods in TransportAction (elastic#31517)
  IndexShard should not return null stats (elastic#31528)
  fix repository update with the same settings but different type (elastic#31458)
  Fix Mockito trying to mock IOException that isn't thrown by method (elastic#31433) (elastic#31527)
  Node selector per client rather than per request (elastic#31471)
  Core: Combine messageRecieved methods in TransportRequestHandler (elastic#31519)
  Upgrade to Lucene 7.4.0. (elastic#31529)
  [ML] Add ML filter update API (elastic#31437)
  Allow multiple unicast host providers (elastic#31509)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants