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

Set the advertisedAddress to pod IP when TLS isn't enabled #198

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lhotari
Copy link
Contributor

@lhotari lhotari commented Apr 27, 2022

  • DNS resolution causes an extra hop for Pulsar topic lookups
  • DNS resolution fails with current settings until the broker's readiness probe succeeds. Pulsar might already return the hostname of a specific broker to a client, but the client cannot resolve the DNS name since the broker's readiness probe hasn't passed. This causes some extra delays when connecting to topics after a load balancing event.
  • broker-deployment already uses a similar solution so it would be consistent to do this also in broker-sts.

Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

I don't think we should merge this feature because it would break namespace affinity rules, which rely on stable broker advertised addresses.

@lhotari
Copy link
Contributor Author

lhotari commented Jun 13, 2022

I don't think we should merge this feature because it would break namespace affinity rules, which rely on stable broker advertised addresses.

@michaeljmarshall I guess you mean "namespace isolation policy"? (pulsar-admin ns-isolation-policy) ?

Perhaps it should be configurable whether IP addresses or DNS names are used. It's very rare to use the "namespace isolation policy" feature.

@lhotari lhotari marked this pull request as draft June 13, 2022 11:38
@michaeljmarshall
Copy link
Member

@lhotari - yes, that is the feature I meant to reference, and I think you're right that it's a rarely used feature. The real problem is that this solution does not work when TLS hostname verification is enabled. I think the primary reason to use a STS is when deploying with TLS. If that isn't a requirement, I think I'd recommend deploying with a Deployment

pgier pushed a commit to pgier/datastax-pulsar-helm-chart that referenced this pull request Jul 13, 2022
…ax#198)

* Added -Dlog4j2.formatMsgNoLookups=true to PULSAR_MANAGER_OPTS

* Bump the chart version to release changes

Co-authored-by: Lari Hotari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants