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 replication metric to 0 when losing leadership #20665

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

jorgemarey
Copy link
Contributor

Fixes #11377 (comment)

When the leader changes the replication metrics on the old leader (now follower) keeps reporting 1. This PR fixes that issue.

@jorgemarey jorgemarey requested a review from a team as a code owner February 16, 2024 11:33
Copy link

This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions.

@github-actions github-actions bot added the meta/stale Automatically flagged for inactivity by stalebot label Apr 17, 2024
agent/consul/leader.go Show resolved Hide resolved
@jorgemarey jorgemarey force-pushed the b-replication-metrics branch 2 times, most recently from 9e3747e to b9ef886 Compare April 26, 2024 20:00
@github-actions github-actions bot removed the meta/stale Automatically flagged for inactivity by stalebot label Apr 27, 2024
@sarahalsmiller sarahalsmiller added backport/1.18 pr/no-metrics-test pr/no-changelog PR does not need a corresponding .changelog entry labels Apr 29, 2024
@rboyer
Copy link
Member

rboyer commented May 1, 2024

Hi there. There's one other place that needs a similar change to fixup the rest of the replicators (like for config entries). For historical reasons there are two versions of the replicators, the ACL ones and the common plumbing for the rest. You fixed the former in this PR so far and the other is in this area: https://github.com/hashicorp/consul/blob/main/agent/consul/replication.go#L156

You can see the pairing of status/index normally being set down here https://github.com/hashicorp/consul/blob/main/agent/consul/leader.go#L797 so you'll just have to repeat the fix in this one place and it should be good.

@jorgemarey
Copy link
Contributor Author

Hi @rboyer, I made the change you suggested.

Copy link

This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions.

@github-actions github-actions bot added the meta/stale Automatically flagged for inactivity by stalebot label Aug 26, 2024
@sarahalsmiller sarahalsmiller added the backport/all Apply backports for all active releases per .release/versions.hcl label Aug 29, 2024
@sarahalsmiller sarahalsmiller enabled auto-merge (squash) August 29, 2024 16:13
@sarahalsmiller sarahalsmiller merged commit d12f9cf into hashicorp:main Aug 29, 2024
58 of 60 checks passed
@hc-github-team-consul-core hc-github-team-consul-core added backport/1.19 Changes are backported to 1.19 backport/ent/1.17 Changes are backported to 1.17 ent backport/ent/1.15 Changes are backported to 1.15 ent backport/ent/1.18 Changes are backported to 1.18 ent labels Aug 29, 2024
@hc-github-team-consul-core
Copy link
Collaborator

📣 Hi @jorgemarey! a backport is missing for this PR [20665] for versions [1.15,1.17,1.18] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

4 similar comments
@hc-github-team-consul-core
Copy link
Collaborator

📣 Hi @jorgemarey! a backport is missing for this PR [20665] for versions [1.15,1.17,1.18] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

📣 Hi @jorgemarey! a backport is missing for this PR [20665] for versions [1.15,1.17,1.18] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

📣 Hi @jorgemarey! a backport is missing for this PR [20665] for versions [1.15,1.17,1.18] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

📣 Hi @jorgemarey! a backport is missing for this PR [20665] for versions [1.15,1.17,1.18] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

philrenaud pushed a commit that referenced this pull request Sep 12, 2024
* Set replication metric to 0 when losing leadership

* Fix replication metrics on replication.go also

---------

Co-authored-by: sarahalsmiller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/all Apply backports for all active releases per .release/versions.hcl backport/ent/1.15 Changes are backported to 1.15 ent backport/ent/1.17 Changes are backported to 1.17 ent backport/ent/1.18 Changes are backported to 1.18 ent backport/1.19 Changes are backported to 1.19 meta/stale Automatically flagged for inactivity by stalebot pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metrics values are 0 in >= 1.10.x instead of NaN like in 1.9.x
5 participants