Skip to content

fix #168 #169

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

Merged
merged 4 commits into from
Jun 24, 2021
Merged

fix #168 #169

merged 4 commits into from
Jun 24, 2021

Conversation

nschad
Copy link
Collaborator

@nschad nschad commented Jun 23, 2021

What this PR does:
removed http-metrics port from every headless service and added ClusterIP service with http-metrics port to every component who before only had an headless service before. Headless services should not be used for anything http related, therefore I think its fine to completely remove that port from the Service.

No more dupe :) because of 2 Service with the same port
image

Which issue(s) this PR fixes:
Fixes #168

Checklist

  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Comment on lines -18 to +19
name: http-metrics
targetPort: http-metrics
name: grpc
targetPort: grpc
Copy link
Contributor

Choose a reason for hiding this comment

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

is this right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there is a new ClusterIP Service which only does http-metrics, so basically I moved the http-metrics port to a new service

Copy link
Contributor

Choose a reason for hiding this comment

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

ok but the distributors were using the http port to connect to ingesters, with this they will use the grpc port, will it work without other changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AndreZiviani where do you got that information? Communication between component is almost always (unless mistaken) exclusively grpc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Besides the Chart's included memberlist Service automatically selects every cortex component with the following label
app.kubernetes.io/part-of: memberlist and then the communication should appear through the 7946 port for memberlist which is exposed by the pods

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the distributor used the ingester headless service to communicate with the ingesters but a quick tcpdump showed traffic on the grpc port, sorry for the confusion

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But why don't you try it out and tell me if its working for you :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Applied here and everything is still working 😄

nschad added 3 commits June 23, 2021 17:39
…erIP service with http-metrics port to every component who before only had an headless service before

Signed-off-by: ShuzZzle <[email protected]>
@nschad nschad marked this pull request as draft June 23, 2021 15:40
@nschad nschad marked this pull request as ready for review June 24, 2021 06:30
@nschad nschad merged commit c3f99b7 into cortexproject:master Jun 24, 2021
@nschad nschad mentioned this pull request Jun 28, 2021
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ingester Ingested Samples Rate is doubled after upgrading to Cortex-helm-chart 0.5.0
3 participants