Node.js client uses master only nodes by default

Elastic Stack Elasticsearch
1

Hey,

a couple of days ago I noticed that our node.js client by default seems to use master only nodes for queries, even though there are dedicated data nodes.

The docs show this default node filter:

function defaultNodeFilter (node) {
  // avoid master only nodes
  if (node.roles.master === true &&
      node.roles.data === false &&
      node.roles.ingest === false) {
    return false
  }
  return true
}

The implementation in the git repo shows this:

function defaultNodeFilter (node: Connection): boolean {
  return true
}

Who is right, and do I have to write my own node filter now, that reflects the documentation? I also opened an issue but haven't gotten any response yet.

Thanks so much and have a nice day!

--Alex

1 Like
2

Hi Alex! Thanks so much for pointing this out. It looks like that implementation change was an oversight during a large rewrite a couple years back. I'll respond over on the GitHub issue when I've got a fix in place. :elasticheart:

2 Likes
3

Hey,

it would be great, if you could also provide some help here for folks who cannot upgrade (seems likely, as the client is only forward compatible according to the docs).

Looking at the example in the docs a node object is passed, that allows to access node.roles. However looking in the source a Connection object is passed that stems from a BaseHTTPConnection and does not seem to know anything about Elasticsearch node roles.

How can I implement that functionality with the current client? How am I supposed to figure out the node role without doing an additional HTTP call to my cluster? I hope I simply missed where to access the node in the typescript definitions.

Thanks so much for your help and looking forward to the fix!

--Alex

4

On closer investigation, it seems roles was a vestigial value from 7.x that partly made its way into 8.x, but is not available when it would actually be useful. You can see that roles is implicitly stripped out right here by omission, before passing the node options on to the transport as ConnectionOptions, which does not support roles, or any other custom metadata.

So, it appears that the better choice would actually be to drop roles from the NodeOptions type and update the documentation so that it's clear that nodeFilter is passed Connection values, not NodeOptions, and always returns true by default.

Given that, it's not obvious what nodeFilter should do by default other than what it already does. Without roles that are explicitly assigned at instantiation, we don't know enough about a node to make any useful choices.

If you have a particular use case in mind for how to use nodeFilter, I'd recommend creating your own custom class that extends Connection with the addition of metadata about your nodes that would appear at the right time.

Let me know if you have more questions! For now I'm going to update the docs to reflect the way the code currently works.

5

Hey,

I am a little confused by your question (and I do not agree to drop node roles at all :slight_smile:

If you have a particular use case in mind for how to use nodeFilter ...

I think the only valid use-case would have been what is already stated in the docs - supporting nodes with different roles to make sure that the master node is not overloaded with queries and does not need to act as a coordinating node, because usually that node is less powerful than a data node.

Other clients offer this feature as well: The python client supports node selectors, the java low level client does via a config option.

Just to clarify my understanding: Does your statement above mean, that you are not planning to support, what also other official clients already offer, as this is a common requirement when sniffing is enabled for on-prem installations to make sure, only data nodes are hit with requests?

Thanks a ton for the clarification and helping out here!

--Alex

1 Like
6

My main goal was to ensure the docs reflected what was actually happening, which is nothing, and to avoid introducing breakage by changing the default behavior of the client unexpectedly between minor or patch versions.

That said, I also wasn't certain how broadly we supported that functionality in other language clients. (I already added a task for myself to create a support matrix to track features like this across clients so it's more obvious in the future. :smile:)

In any case, as I sit with the idea, it occurs to me that roles being a value that is explicitly set by the end user of the client, and nodeFilter not going into effect unless roles is set, it's probably not very likely that reintroducing the functionality would break anything. And, even if it did "break," the client will still work, it would just be sending requests to fewer nodes than it was before, and avoiding nodes that shouldn't have been receiving the traffic anyway.

I'll reintroduce the behavior soon and will notify on the GitHub issue when it's released.

7

Hey Josh,

nice, thanks for your work and looking forward to the fix!

--Alex

1 Like