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

Java-REST-Client NodeSniffer Fails parsing publish_address properly #48950

Closed
UglyBarnacle opened this issue Nov 11, 2019 · 12 comments
Closed

Java-REST-Client NodeSniffer Fails parsing publish_address properly #48950

UglyBarnacle opened this issue Nov 11, 2019 · 12 comments
Labels
>bug good first issue low hanging fruit

Comments

@UglyBarnacle
Copy link

Elasticsearch version (bin/elasticsearch --version): 7.4.2 - Java Client 7.4.2

Plugins installed: []

JVM version (java -version): jdk11

OS version (uname -a if on a Unix-like system): unix docker

Description of the problem including expected versus actual behavior:
Node-Sniffer for Java-Rest-Client fails parsing the new hostname/ip:port format properly.
Client is initalized with a consul-address containing the port. instead the hostname is just used with port 80, the sniff() fails with an ConnectionException.
=> expected IP and port are extracted 192.168.0.185:9202
=> actual only hostname is parsed integration-stack-01.com.pany

Steps to reproduce:

Please include a minimal but complete recreation of the problem, including
(e.g.) index creation, mappings, settings, query etc. The easier you make for
us to reproduce it, the more likely that somebody will take the time to look at it.

  1. setup clusternode with hostname so publish_address : "integration-stack-01.com.pany/192.168.0.185:9202"
  2. create simple main-Class Java starting a Client and Sniffer
  3. Sniffer fails at sniff() because address is parsed not properly

Provide logs (if relevant):

GET http://elasticsearch3.service.int.consul:9202/_nodes/http?pretty
{
  "_nodes" : {
    "total" : 3,
    "successful" : 3,
    "failed" : 0
  },
  "cluster_name" : "company-int-3",
  "nodes" : {
    "qGSyScvjRK2jbujcpo9t3g" : {
      "name" : "integration-stack-01",
      "transport_address" : "192.168.0.185:9302",
      "host" : "integration-stack-01.com.pany",
      "ip" : "192.168.0.185",
      "version" : "7.4.2",
      "build_flavor" : "default",
      "build_type" : "tar",
      "build_hash" : "2f90bbf7b93631e52bafb59b3b049cb44ec25e96",
      "roles" : [
        "ingest",
        "master",
        "data",
        "ml"
      ],
      "attributes" : {
        "ml.machine_memory" : "67498500096",
        "ml.max_open_jobs" : "20"
      },
      "http" : {
        "bound_address" : [
          "[::]:9202"
        ],
        "publish_address" : "integration-stack-01.com.pany/192.168.0.185:9202",
        "max_content_length_in_bytes" : 104857600
      }
    },
	...

Error:

16:46:03.278 [pool-1-thread-1] DEBUG org.apache.http.impl.nio.client.InternalHttpAsyncClient - [exchange: 13] connection request failed
16:46:03.278 [pool-1-thread-1] DEBUG org.elasticsearch.client.RestClient - request [GET http://integration-stack-03.com.pany/_nodes/http?timeout=1000ms] failed

Example:

package com.pany.elasticsearchrestclients;

import org.apache.http.HttpHost;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.client.RestClientBuilder;
import org.elasticsearch.client.RestHighLevelClient;
import org.elasticsearch.client.sniff.SniffOnFailureListener;
import org.elasticsearch.client.sniff.Sniffer;

public class TestStarter
{
	public static void main(final String[] args)
	{
		SniffOnFailureListener sniffOnFailureListener = new SniffOnFailureListener();
		RestClientBuilder builder = RestClient.builder(new HttpHost("elasticsearch3.service.int.consul", 9202)).setFailureListener(sniffOnFailureListener);
		RestHighLevelClient client = new RestHighLevelClient(builder);
		Sniffer sniffer = Sniffer.builder(client.getLowLevelClient()).setSniffAfterFailureDelayMillis(1_000).setSniffIntervalMillis(500).build();
		sniffOnFailureListener.setSniffer(sniffer);
	}
}

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Java High Level REST Client)

@hub-cap
Copy link
Contributor

hub-cap commented Nov 12, 2019

Hi, can you clarify a few things here. Are you saying that the following line does not work?

		RestClientBuilder builder = RestClient.builder(new HttpHost("elasticsearch3.service.int.consul", 9202)).setFailureListener(sniffOnFailureListener);

When you say

Node-Sniffer for Java-Rest-Client fails parsing the new hostname/ip:port format properly.

What is giving out this address? is it consul itself?

@UglyBarnacle
Copy link
Author

		RestClientBuilder builder = RestClient.builder(new HttpHost("elasticsearch3.service.int.consul", 9202)).setFailureListener(sniffOnFailureListener);

initalizing the client works seamlessly, it connects with the provided consul address to the cluster, connection established ( and connection stays if no sniffer is used )

Node-Sniffer for Java-Rest-Client fails
parsing the new hostname/ip:port format properly.

then the Node Sniffer kicks in and does its round, parsing the response from the nodes endpoint. here it receives the publish_address in the es7 format hostname/ip:port.
the json parser inside the node sniffer parses the value into a String and creates a new URI - here it fails, everything after the slash is considered as a path, the port is neglected and an URI with hostname and port 80 (-1) is created. the sniffer returns the new found nodes, updates the client, no connection anymore.

I will provide the settings file of our cluster if necessary.

@hub-cap
Copy link
Contributor

hub-cap commented Nov 12, 2019

Oh no, i didnt scroll down, i see what you are saying now. The

        "publish_address" : "integration-stack-01.com.pany/192.168.0.185:9202",

is different so its failing, correct?

@hub-cap
Copy link
Contributor

hub-cap commented Nov 12, 2019

So it looks like #32806 introduced a change in the publish_address field which is causing the issue here. Its definitely a bug and could be fixed here https://github.com/elastic/elasticsearch/blob/master/client/sniffer/src/main/java/org/elasticsearch/client/sniff/ElasticsearchNodesSniffer.java#L166-L169

This is a great first issue for someone to fix.

@hub-cap hub-cap added >bug good first issue low hanging fruit labels Nov 12, 2019
@zacharymorn
Copy link
Contributor

Hi @hub-cap I can take a look at this one.

@hub-cap
Copy link
Contributor

hub-cap commented Nov 14, 2019

Awesome @zacharymorn! Just so you know, it looks like the publish_address can come back in either format, so be sure to check if it is one format or the other so it will work for both cases. Thank you for picking it up!

@zacharymorn
Copy link
Contributor

@hub-cap I've created a pull request to support both formats. Please let me know if this looks good to you.

@hub-cap
Copy link
Contributor

hub-cap commented Dec 3, 2019

awesome, great work. Ive been on vacation and it looks like @jbaiera is reviewing the other PR. Ill sync w him to see if he needs anything from me.

@jbaiera
Copy link
Member

jbaiera commented Dec 3, 2019

Also been on vacation. The other PR is on my radar for today. Thanks for your patience!

@zacharymorn
Copy link
Contributor

Sure no problem, thanks for reviewing the changes!

@jrodewig
Copy link
Contributor

Closing as #49279 is now merged. Please re-open if more work or discussion is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug good first issue low hanging fruit
Projects
None yet
Development

No branches or pull requests

7 participants