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 GH-16932 Scoreboard reset at end of request #16941

Closed
wants to merge 1 commit into from

Conversation

lucasnetau
Copy link
Contributor

@lucasnetau lucasnetau commented Nov 26, 2024

Scoreboard was updated with FPM_SCOREBOARD_ACTION_SET in PR#14153 causing all metrics to be reset at the end of request. Use FPM_SCOREBOARD_ACTION_INC instead to ensure we only update peak memory

Fixes #16932

@lucasnetau lucasnetau requested a review from bukka as a code owner November 26, 2024 09:46
@cmb69 cmb69 linked an issue Nov 26, 2024 that may be closed by this pull request
@bukka
Copy link
Member

bukka commented Nov 27, 2024

I just created a PR that includes a test. I would normally comment here and try to guide you to create the test but 8.4.2 will be branched early next week and I really wanted to get it there. I will for sure give you a credit in NEWS (changelog) and will also add Co-authored-by so you get some credit.

I actually chose keeping the SET because I think it's semantically better (the memory is actually being set - I know that it's set in any case but seeing INC might be a bit more confusing).

So closing in favour of #16974

@bukka bukka closed this Nov 27, 2024
@lucasnetau
Copy link
Contributor Author

No problems @bukka, thanks for doing the test, I didn't know my way around the fpm tester, will remember that in the future. Fine with keeping at SET, semantically it makes more sense than INC, I went with the minimal change approach when making the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8.4 wrong fpmstatus output
2 participants