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

Fix metrics not shutting down if there are open connections #6220

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

tmpolaczyk
Copy link
Contributor

Fix prometheus metrics not shutting down if there are open connections. I fixed the same issue in the past but it broke again after a dependecy upgrade.

See also:

#1637

@bkchr
Copy link
Member

bkchr commented Oct 24, 2024

I fixed the same issue in the past but it broke again after a dependecy upgrade.

Do we want to prevent this again from happening? By introducing a test? :D

@bkchr bkchr added the T0-node This PR/Issue is related to the topic “node”. label Oct 24, 2024
@bkchr
Copy link
Member

bkchr commented Oct 24, 2024

@tmpolaczyk also please provide some prdoc.

@bkchr bkchr requested a review from a team October 24, 2024 19:46
@tmpolaczyk
Copy link
Contributor Author

@bkchr not sure if this is easy to test. I have a zombienet test in our project that will fail if this breaks, but it takes 5 minutes to run.

For a unit test we would need to:

  • Start metrics on port N
  • Create tcp client and connect to port N
  • Stop metrics
  • Try to start something else on port N, will fail because the port is still being used

@@ -0,0 +1,10 @@
title: Improve `CheckMetadataHash` transaction extension weight and logic
Copy link
Contributor

Choose a reason for hiding this comment

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

title needs to be updated, to reference the actual fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙈 thanks

@girazoki
Copy link
Contributor

@bkchr is there anything else to do before merging this PR?

@bkchr bkchr added this pull request to the merge queue Nov 19, 2024
github-merge-queue bot pushed a commit that referenced this pull request Nov 19, 2024
Fix prometheus metrics not shutting down if there are open connections.
I fixed the same issue in the past but it broke again after a dependecy
upgrade.

See also:

#1637
@bkchr
Copy link
Member

bkchr commented Nov 19, 2024

No! Sorry for forgetting! 😅

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 19, 2024
@bkchr bkchr added this pull request to the merge queue Nov 19, 2024
Merged via the queue into paritytech:master with commit 0449b21 Nov 19, 2024
190 of 194 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants