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

ISPN-13982: Expand content on setting BASIC intelligence for cluster topology #1592

Merged
merged 6 commits into from Jul 11, 2022

Conversation

dfitzmau
Copy link
Contributor

@dfitzmau dfitzmau commented Jul 1, 2022

@dfitzmau
Copy link
Contributor Author

dfitzmau commented Jul 1, 2022

Hello @wfink . Would you be OK to review? This PR relates to #1592, but with the focus on the Infinispan Operator guide.

@dfitzmau dfitzmau force-pushed the ISPN-13982 branch 2 times, most recently from c74936a to 47bdacc Compare July 6, 2022 11:58
@dfitzmau dfitzmau force-pushed the ISPN-13982 branch 2 times, most recently from 63844da to 7e374dd Compare July 6, 2022 16:17
@ryanemerson ryanemerson self-requested a review July 7, 2022 16:20
Copy link
Contributor

@ryanemerson ryanemerson left a comment

Choose a reason for hiding this comment

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

Content LGTM. The commit message has been lost though, could you update and repush @dfitzmau.

I'll wait for the doc reviewers to accept before merging the PR.


[NOTE]
====
{k8s} cluster administrators can define network policies that restrict traffic to {brandname}.
In some cases network isolation policies can require you to use `BASIC` intelligence even when clients are running in the same {k8s} cluster but a different namespace.
====

Hot Rod clients must use `BASIC` intelligence when connecting to {brandname} through a `LoadBalancer`, `NodePort`, or {openshiftshort} `Route`.
`HASH_DISTRIBUTION_AWARE`:: The default intelligence mechanism. Best mechanism for allowing clients to route requests to primary owners, which improves a client's performance.
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking about something similar to https://github.com/infinispan/infinispan/pull/10200/files which also organizes the text using some kind of heading.
HASH_DISTRIBUTION_AWARE:: when to use it and why
BASIC:: when to use it and why
But it only makes sense to use it for both terms. If you want to keep it as it is then drop HASH_DISTRIBUTION_AWARE:: and move the sentence back to line 13

Copy link
Contributor Author

@dfitzmau dfitzmau Jul 8, 2022

Choose a reason for hiding this comment

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

Thanks, @dvagnero .

@ryanemerson and @wfink , do you think I should expand this file to align with the structure in https://github.com/infinispan/infinispan/pull/10200/files ? Maybe I should raise a separate PR to conduct this work because it's out of scope of the request Jira request?

@dfitzmau dfitzmau force-pushed the ISPN-13982 branch 2 times, most recently from f540a3b to 540e9d8 Compare July 8, 2022 09:28
@dfitzmau
Copy link
Contributor Author

Hi, @dvagnero and @oraNod . Might I ask which one of you wants to complete the approval of this PR?

@oraNod
Copy link
Collaborator

oraNod commented Jul 11, 2022

@dfitzmau Yes, I'll approve and merge. Looks fine now but I believe there are additional empty lines at the end of the file or is that just me?

@dfitzmau
Copy link
Contributor Author

@dfitzmau Yes, I'll approve and merge. Looks fine now but I believe there are additional empty lines at the end of the file or is that just me?

Yes, you're correct. I have not noticed this too much when I used Atom as my editor. VSC clearly treats empty lines differently. I removed these lines. Thanks!

@oraNod oraNod self-requested a review July 11, 2022 09:59
Copy link
Collaborator

@oraNod oraNod left a comment

Choose a reason for hiding this comment

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

LGTM

@oraNod oraNod merged commit f9c4e08 into infinispan:main Jul 11, 2022
@oraNod
Copy link
Collaborator

oraNod commented Jul 11, 2022

Thanks @dfitzmau Can you create the backport for 2.2?

@dfitzmau
Copy link
Contributor Author

Thanks @dfitzmau Can you create the backport for 2.2?

Thanks, @oraNod . I'll converse with Dominika on how I can do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Adds or changes docu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants