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

fix: pull queries should work across nodes #4169

Merged
merged 4 commits into from
Dec 20, 2019

Conversation

big-andy-coates
Copy link
Contributor

@big-andy-coates big-andy-coates commented Dec 18, 2019

Description

Fixes: #4142
Fixes: #4151
Fixes: #4152

Introduces a new inter.node.listener that can be used to specify a URL that the node can be contacted on by other nodes. This can be different to the listeners defined in listeners. This can be required if listeners is set to a wildcard address, i.e. IPv4 0.0.0.0 or IPv6 [::], or if the node sits behind network infrastructure that requires other nodes to reach it using a different URL.

If inter.node.listener is not set it still defaults to the first listener in listener config. However, it now replaces an wildcard address with localHost. This means inter-node comms is still possible for nodes running on the same host.

Warnings are logged if the inter-node listener resolves to a loopback or local address.

Testing done

Usual.

Manual testing using two node docker KSQL cluster, using KSQL_LISTENERS: http://0.0.0.0:8088: the wildcard address is correctly replaced with localhost when used to share with other nodes. Optionally, can set KSQL_ADVERTISED_LISTENER to expose KSQL beyond local machine.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

Fixes: confluentinc#4142
Fixes: confluentinc#4151
Fixes: confluentinc#4152

Introduces a new `inter.node.listener` that can be used to specify a URL that the node can be contacted on by other nodes.  This can be different to the listeners defined in `listeners`. This can be required if `listeners` is set to a wildcard address, i.e. IPv4 `0.0.0.0` or IPv6 `[::]`, or if the node sits behind network infrastructure that requires other nodes to reach it using a different URL.

If `inter.node.listener` is not set it still defaults to the first listener in `listener` config. However, it now replaces an wildcard address with `localHost`. This means inter-node comms is still possible for nodes running on the same host.

Warnings are logged if the inter-node listener resolves to a loopback or local address.
@big-andy-coates big-andy-coates requested a review from a team as a code owner December 18, 2019 15:32
Conflicting files
ksql-rest-app/src/main/java/io/confluent/ksql/rest/server/KsqlRestConfig.java
@AlanConfluent AlanConfluent self-assigned this Dec 18, 2019
@vpapavas
Copy link
Member

Hey @big-andy-coates , should the inter.node.listener be the same as the streams config application.server?

@big-andy-coates
Copy link
Contributor Author

Hey @big-andy-coates , should the inter.node.listener be the same as the streams config application.server?

It's potentially used to set application.server

@big-andy-coates big-andy-coates requested review from AlanConfluent and a team December 19, 2019 13:00
@big-andy-coates big-andy-coates merged commit 0ac71cf into confluentinc:master Dec 20, 2019
@big-andy-coates big-andy-coates deleted the intra_node branch December 20, 2019 12:02
big-andy-coates added a commit to big-andy-coates/ksql that referenced this pull request Jan 10, 2020
Backport of confluentinc#4169

Fixes: confluentinc#4142
Fixes: confluentinc#4151
Fixes: confluentinc#4152

Introduces a new `inter.node.listener` that can be used to specify a URL that the node can be contacted on by other nodes.  This can be different to the listeners defined in `listeners`. This can be required if `listeners` is set to a wildcard address, i.e. IPv4 `0.0.0.0` or IPv6 `[::]`, or if the node sits behind network infrastructure that requires other nodes to reach it using a different URL.

If `inter.node.listener` is not set it still defaults to the first listener in `listener` config. However, it now replaces an wildcard address with `localHost`. This means inter-node comms is still possible for nodes running on the same host.

Warnings are logged if the inter-node listener resolves to a loopback or local address.

(cherry picked from commit 0ac71cf)
@big-andy-coates
Copy link
Contributor Author

Backported to 5.4.x in #4271. (Should be released in 5.4.1)

big-andy-coates added a commit that referenced this pull request Jan 26, 2020
Backport of #4169

Fixes: #4142
Fixes: #4151
Fixes: #4152

Introduces a new `inter.node.listener` that can be used to specify a URL that the node can be contacted on by other nodes.  This can be different to the listeners defined in `listeners`. This can be required if `listeners` is set to a wildcard address, i.e. IPv4 `0.0.0.0` or IPv6 `[::]`, or if the node sits behind network infrastructure that requires other nodes to reach it using a different URL.

If `inter.node.listener` is not set it still defaults to the first listener in `listener` config. However, it now replaces an wildcard address with `localHost`. This means inter-node comms is still possible for nodes running on the same host.

Warnings are logged if the inter-node listener resolves to a loopback or local address.

(cherry picked from commit 0ac71cf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants