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.

Sign up for GitHub

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

Jump to bottom

Target Allocator multiple Replicas with per-node allocationStrategy #2900

Open
diezfx opened this issue Apr 26, 2024 · 5 comments · May be fixed by #2932
Open

Target Allocator multiple Replicas with per-node allocationStrategy #2900

diezfx opened this issue Apr 26, 2024 · 5 comments · May be fixed by #2932
Labels
area:target-allocator Issues for target-allocator needs triage question Further information is requested

Comments

@diezfx
Copy link

diezfx commented Apr 26, 2024

Component(s)

No response

Describe the issue you're reporting

Component(s)

target allocator

Description

The documentation says:

Replicas is the number of pod instances for the underlying TargetAllocator. This should only be set to a value other than 1 if a strategy that allows for high availability is chosen. Currently, the only allocation strategy that can be run in a high availability mode is consistent-hashing.

Is there a reason the per-node allocation does not support multiple replicas currently?
In contrast to least-weighted, there should be no problems with contradicting allocations.
@pavolloffay pavolloffay added question Further information is requested area:target-allocator Issues for target-allocator labels Apr 26, 2024
@pavolloffay
Copy link
Member

cc) @open-telemetry/operator-ta-maintainers

@jaronoff97
Copy link
Contributor

This should work for multiple replicas as stated, but I haven't tested this yet. cc @matej-g @swiatekm-sumo have you tested this? Or do we have confidence this should just work?

@swiatekm-sumo
Copy link
Contributor

I've not tested it, but it should work correctly. At the very least, we don't check Replicas at admission, so you can really set it to whatever you want. If we wanted to test it, what we should really do is add unit tests for strategies where we randomly rearrange input targets and assert that the allocation is always the same.

@diranged
Copy link

So you can actually run multiple replicas, but you are not allowd to set a PDB .. which isn't right IMO. See my thread in slack from just a few days ago discussing this: https://cloud-native.slack.com/archives/C02J58HR58R/p1714147449298209

@swiatekm-sumo
Copy link
Contributor

Yeah, you should be able to set a PDB, and the default PDB should be set for the per-node strategy.

@jaronoff97 jaronoff97 linked a pull request May 7, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:target-allocator Issues for target-allocator needs triage question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow setting of PDB for per-node jaronoff97/opentelemetry-operator
5 participants
@diranged @pavolloffay @jaronoff97 @diezfx @swiatekm-sumo