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

Need the option to enable/disable Session History Logging #167

Closed
charlescapps opened this issue Oct 7, 2015 · 5 comments
Closed

Need the option to enable/disable Session History Logging #167

charlescapps opened this issue Oct 7, 2015 · 5 comments

Comments

@charlescapps
Copy link

Any major feature of SGE that can impact performance must be configurable.

I recently did a heap dump, and discovered that the biggest objects in memory for our Selenium standalone process on our Hub are due to the com.groupon.seleniumgridextras.loggers.SessionHistoryLog data.

Here's a screenshot of the heapdump
screen shot 2015-10-07 at 10 26 38 am

So, I will add a feature so that the call in SetupTeardownProxy to create a new SessionHistoryCallable is configurable.

@smccarthy
Copy link
Collaborator

I hope to look at your PR by this weekend : #168
Thank you.

@charlescapps
Copy link
Author

Thanks! Yes, this is a simple idea to allow turning off the session logging. I found that our hub's memory usage went down about 1 GB after using this.

In the long run, we could figure out why session logging is using so much memory. It's probably just that a lot of logs can be generated when you have a lot of tests!

I think the best solution is to not keep the logs in memory. We could just use log4j or another out-of-the-box solution for logging sessions, not sure what the benefit is of keeping the data in memory.

@lucacri
Copy link

lucacri commented Oct 20, 2015

+1 for this!

smccarthy added a commit that referenced this issue Oct 20, 2015
…rable

#167 - Allow turning on/off the SessionHistory feature.
@smccarthy
Copy link
Collaborator

@charlescapps I think you wrote a PR for this, that was merged in. Can this be closed?

@charlescapps
Copy link
Author

Yes, my bad, I had forgotten to close this. Thanks so much for merging the PR!

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

No branches or pull requests

3 participants