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

Remove instperf #5299

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Conversation

M4rtinK
Copy link
Contributor

@M4rtinK M4rtinK commented Nov 6, 2023

This was added ~12 years ago in 4e5940f as an attempt to record memory usage & identify cause of OOM situations.

Unfortunately it is not documented anywhere and as far as I can tell (asking others in the team + Fedora QA/RTT) not used in at least a couple years.

So lets drop this & cleanup the source tree and our systemd targets and makefiles a bit.

This was added ~12 years ago in 4e5940f
as an attempt to record memory usage & identify cause of OOM situations.

Unfortunately it is not documented anywhere and as far as I can tell
(asking others in the team + Fedora QA/RTT) not used in at least
a couple years.

So lets drop this & cleanup the source tree and our systemd targets
and makefiles a bit.
@M4rtinK M4rtinK added the f40 label Nov 6, 2023
@M4rtinK
Copy link
Contributor Author

M4rtinK commented Nov 6, 2023

/kickstart-test --testtype smoke

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

LGTM!

@M4rtinK M4rtinK merged commit bed2e8b into rhinstaller:master Nov 9, 2023
18 checks passed
@AdamWill
Copy link
Contributor

Um! We do use this. We have a test in openQA - the memory_check test - which boots with debug and uploads memory.dat, and the check-compose tool which generates the compose check reports parses that data and flags if memory use increases.

We have also used this in the past just manually to try and debug excessive memory usage by the installer.

@M4rtinK
Copy link
Contributor Author

M4rtinK commented Nov 21, 2023

Um! We do use this. We have a test in openQA - the memory_check test - which boots with debug and uploads memory.dat, and the check-compose tool which generates the compose check reports parses that data and flags if memory use increases.

We have also used this in the past just manually to try and debug excessive memory usage by the installer.

I think I asked @kparal or someone else from the Brno section of Fedora QA about instperf & it looked like its not being used. Maybe I should have better worded it - something like memory usage profiling rather than the obscure name. Oops! :P

So I guess we should put it back, right ? :)

@AdamWill
Copy link
Contributor

ideally yeah, if it's not too much of a maintenance burden. It's not critical, or anything, but it has been useful on occasion.

@AdamWill
Copy link
Contributor

As expected, this has broken the openQA test.

@KKoukiou KKoukiou deleted the master-remove-instperf branch January 31, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 participants