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

Throw an exception when a Transport Client is not part of the connected cluster #4569

Closed
YannBrrd opened this issue Dec 30, 2013 · 6 comments
Labels

Comments

@YannBrrd
Copy link

Hi,

It would be really nice to let developers know when a Transport Client is not connected to the right cluster, instead of simply logging a WARN exception.

Developers should be able to handle that on their own, and decided whether it is a warning or an error.

Thanks.
Yann

kimchy added a commit that referenced this issue Dec 30, 2013
Expose the list of nodes that were filtered out with the TransportClient, for example, due to different cluster name. Relates to #4569
closes #4571
kimchy added a commit that referenced this issue Dec 30, 2013
Expose the list of nodes that were filtered out with the TransportClient, for example, due to different cluster name. Relates to #4569
closes #4571
brusic pushed a commit to brusic/elasticsearch that referenced this issue Jan 19, 2014
Expose the list of nodes that were filtered out with the TransportClient, for example, due to different cluster name. Relates to elastic#4569
closes elastic#4571
@javanna
Copy link
Member

javanna commented Mar 28, 2015

Hi @YannBrrd sorry it took ages to get back to you on this. We discussed this and we think it's a valid improvement. Throwing exceptions when there's a problem instead of only logging it seems like a good idea in general.

Given that the transport client supports connecting to multiple nodes though, I wonder when we should throw an exception. I guess we should do it when all of the nodes get filtered out after the cluster name check, cause anyways we would get a NoNodeAvailableException at the first request performed using the client. But what about when only some of the nodes get filtered out, but some (at least one) are valid ones that we can connect to? In that case warning seems ok to me. Let me know what you think.

Marking as adoptme as this is something we would like to get in.

@javanna javanna added help wanted adoptme good first issue low hanging fruit >enhancement :Core/Infra/Transport API Transport client API and removed discuss labels Mar 28, 2015
@YannBrrd
Copy link
Author

Hi,

Fully agree with the approach. What is misleading as for now is having a
client running without any connection set (which may be because of client
conf or cluster unavailable). Having an exception thrown them is a good
idea to inform the client.

Then if some node are unavailable, I'd also send an exception informing how
many are down, letting client decide to go on our not...

How this would behave in case of brain split for example ?

Cheers,
Yann

Le sam. 28 mars 2015 09:39, Luca Cavanna notifications@github.com a
écrit :

Hi @YannBrrd https://github.com/YannBrrd sorry it took ages to get back
to you on this. We discussed this and we think it's a valid improvement.
Throwing exceptions when there's a problem instead of only logging it seems
like a good idea in general.

Given that the transport client supports connecting to multiple nodes
though, I wonder when we should throw an exception. I guess we should do it
when all of the nodes get filtered out after the cluster name check, cause
anyways we would get a NoNodeAvailableException at the first request
performed using the client. But what about when only some of the nodes get
filtered out, but some (at least one) are valid ones that we can connect
to? In that case warning seems ok to me. Let me know what you think.

Marking as adoptme as this is something we would like to get in.


Reply to this email directly or view it on GitHub
#4569 (comment)
.

@javanna
Copy link
Member

javanna commented Mar 28, 2015

The transport client checks the cluster name to make sure that it's connecting to the right cluster, and that is why you see the warning on the log when some nodes get filtered out because the cluster name doesn't match. This doesn't have anything to do with split brain situations, where the cluster name is correct and stays the same, but the different splits have different masters. Makes sense?

I am not sure about throwing exception when we can connect to at least one node, but some belong to a different cluster. Seems like we can make things work and nothing bad happens as we will connect to a single cluster anyways. I am for only warning in that case.

By the way you could potentially double check in the client which nodes get filtered out, if any, and act accordingly (e.g. throw yourself an exception), have a look here.

@YannBrrd
Copy link
Author

Ok, makes sense.
Thanks.

Cordialement,
Yann Barraud

2015-03-28 10:31 GMT+01:00 Luca Cavanna notifications@github.com:

The transport client checks the cluster name to make sure that it's
connecting to the right cluster, and that is why you see the warning on the
log when some nodes get filtered out because the cluster name doesn't
match. This doesn't have anything to do with split brain situations, where
the cluster name is correct and stays the same, but the different splits
have different masters. Makes sense?

I am not sure about throwing exception when we can connect to at least one
node, but some belong to a different cluster. Seems like we can make things
work and nothing bad happens as we will connect to a single cluster
anyways. I am for only warning in that case.

By the way you could potentially double check in the client which nodes
get filtered out, if any, and act accordingly (e.g. throw yourself an
exception), have a look here
https://github.com/elastic/elasticsearch/blob/master/src/main/java/org/elasticsearch/client/transport/TransportClient.java#L220
.


Reply to this email directly or view it on GitHub
#4569 (comment)
.

mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
Expose the list of nodes that were filtered out with the TransportClient, for example, due to different cluster name. Relates to elastic#4569
closes elastic#4571
@clintongormley
Copy link

@javanna is this still an issue?

@javanna
Copy link
Member

javanna commented Nov 7, 2016

no @clintongormley I have re-read the discussion and I don't think there's anything left to be done here. If there are no nodes to connect to, an exception is thrown. If some nodes are filtered out they can be programmatically retrieved through the filteredNodes method besides the warning that gets logged.

@javanna javanna closed this as completed Nov 7, 2016
@javanna javanna removed the help wanted adoptme label Nov 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants