Skip to content

OAK-10710 : added option to reset detailedGC after finishing detailed…#1367

Merged
rishabhdaim merged 1 commit intoDetailedGC/OAK-10199from
OAK-10710
Mar 14, 2024
Merged

OAK-10710 : added option to reset detailedGC after finishing detailed…#1367
rishabhdaim merged 1 commit intoDetailedGC/OAK-10199from
OAK-10710

Conversation

@rishabhdaim
Copy link
Contributor

…GC cycle

Copy link
Contributor

@stefan-egli stefan-egli left a comment

Choose a reason for hiding this comment

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

looks ok, some things confuse slightly, maybe you could explain: the removal of the detailedGC / enable detailedGC (collect only) part, is that related to the reset or is it just coincidence and a general cleanup? also I was wondering how useful this new reset-after-full-repo-detailedGC is - I have no concerns having it - just is it really so useful? Couldn't you otherwise just call the reset command only (that would, ok, have to start up oak-run again and connect to mongo for that, but that's it, it would take maybe 10seconds - is it about saving those 10sec? just to understand). Otherwise just wondering if we should in the future aim to do general improvement of code (which I agree we should indeed do) in different PRs (refering to the removal of the guava list thing - which I agree which should do).

@rishabhdaim
Copy link
Contributor Author

rishabhdaim commented Mar 14, 2024

the removal of the detailedGC / enable detailedGC (collect only) part

  • This is done to avoid confusion since we have 2 ways to run detailedGC code i.e. from collect & detailedGC command.

I was wondering how useful this new reset-after-full-repo-detailedGC

  • I agree that this can be achieved by calling reset command, but having it inside the detailedGC command gives us one place/command to control everything around detailedGC. Also, it will save both the time & effort of a developer.

Otherwise just wondering if we should in the future aim to do general improvement of code

  • I did it since it was only one-liner.

P.S. thanks for the quick review @stefan-egli

@rishabhdaim rishabhdaim merged commit 6e393c6 into DetailedGC/OAK-10199 Mar 14, 2024
@rishabhdaim rishabhdaim deleted the OAK-10710 branch March 14, 2024 18:57
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.

2 participants