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

[DS-2846] Optimize memory usage when using iterators #1131

Closed

Conversation

KevinVdV
Copy link
Member

https://jira.duraspace.org/browse/DS-2846
Use a scrollable result when returning an iterator, this will decrease changes of getting a heap error.

@KevinVdV KevinVdV added the bug label Oct 28, 2015
@KevinVdV KevinVdV added this to the 6.0 milestone Oct 28, 2015
@tdonohue
Copy link
Member

This code currently fails unit tests. The errors seem to always be along the lines of

org.hibernate.PessimisticLockException: could not execute statement
...
Caused by: org.h2.jdbc.JdbcSQLException: Timeout trying to lock table ; SQL statement:
update eperson set can_log_in=?, digest_algorithm=?, email=?, last_active=?, netid=?, password=?, require_certificate=?, salt=?, self_registered=? where uuid=? [50200-187]
...
Caused by: org.h2.jdbc.JdbcSQLException: Concurrent update in table "EPERSON": another transaction has updated or deleted the same row [90131-187]

@KevinVdV
Copy link
Member Author

Fleshed out the iterator implementation a bit and this appears to fix the unit tests, but @terrywbrady will need to test if the OutOfMemory problems are solved now.

@terrywbrady
Copy link
Contributor

@KevinVdV, sorry for the delay in testing this. I concluded that my MaxPermSize for tomcat was set too low. That may have explained some of my issues. I will test this with a larger database instance after I find a resolution for https://jira.duraspace.org/browse/DS-2960

@terrywbrady
Copy link
Contributor

@KevinVdV , Since you resolved DS-2960/DS-2913, the behavior of my repository has improved significantly. It has been difficult to isolate the issue behind this PR.

I have tested with this change and things look good in my repository. +1

@terrywbrady
Copy link
Contributor

REST and XMLUI are behaving well.

I initiated a re-index of my repository with 250,000 items. That code encountered a heap issue.

I will plan to dig deeper into this issue.

...bin/dspace index-discovery -b
Exception: Java heap space
java.lang.OutOfMemoryError: Java heap space
        at org.hibernate.engine.internal.EntityEntryContext.reentrantSafeEntityEntries(EntityEntryContext.java:213)
        at org.hibernate.engine.internal.StatefulPersistenceContext.reentrantSafeEntityEntries(StatefulPersistenceContext.java:1287)
        at org.hibernate.event.internal.AbstractFlushingEventListener.prepareEntityFlushes(AbstractFlushingEventListener.java:151)
        at org.hibernate.event.internal.AbstractFlushingEventListener.flushEverythingToExecutions(AbstractFlushingEventListener.java:89)
        at org.hibernate.event.internal.DefaultAutoFlushEventListener.onAutoFlush(DefaultAutoFlushEventListener.java:61)
        at org.hibernate.internal.SessionImpl.autoFlushIfRequired(SessionImpl.java:1166)
        at org.hibernate.internal.SessionImpl.list(SessionImpl.java:1616)
        at org.hibernate.internal.CriteriaImpl.list(CriteriaImpl.java:374)
        at org.dspace.core.AbstractHibernateDAO.list(AbstractHibernateDAO.java:114)
        at org.dspace.authorize.dao.impl.ResourcePolicyDAOImpl.findByDSoAndAction(ResourcePolicyDAOImpl.java:69)
        at org.dspace.authorize.ResourcePolicyServiceImpl.find(ResourcePolicyServiceImpl.java:104)
        at org.dspace.authorize.AuthorizeServiceImpl.getPoliciesActionFilter(AuthorizeServiceImpl.java:478)
        at org.dspace.discovery.SolrServiceResourceRestrictionPlugin.additionalIndex(SolrServiceResourceRestrictionPlugin.java:47)
        at org.dspace.discovery.SolrServiceImpl.buildDocument(SolrServiceImpl.java:1393)
        at org.dspace.discovery.SolrServiceImpl.indexContent(SolrServiceImpl.java:216)
        at org.dspace.discovery.SolrServiceImpl.updateIndex(SolrServiceImpl.java:395)
        at org.dspace.discovery.SolrServiceImpl.createIndex(SolrServiceImpl.java:356)
        at org.dspace.discovery.IndexClient.main(IndexClient.java:117)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:606)
        at org.dspace.app.launcher.ScriptLauncher.runOneCommand(ScriptLauncher.java:226)
        at org.dspace.app.launcher.ScriptLauncher.main(ScriptLauncher.java:78)

@KevinVdV
Copy link
Member Author

KevinVdV commented Feb 5, 2016

@terrywbrady In order to fix: #1276 did you also require this pull request or was yours enough ?

If yours was enough, does this PR have an influence on performance ?

@terrywbrady
Copy link
Contributor

@KevinVdV , sorry that I overlooked your last comment.

Testing without this enhancement, a re-index of 250,000 items took 3:08 hours.

I will repeat the test with this code merged and post the time to complete the task.

@terrywbrady
Copy link
Contributor

@KevinVdV , with this pr, my re-index took 3:25 min. Based on this single test, I do not believe that this PR improves indexing performance.

@terrywbrady terrywbrady removed their assignment Mar 3, 2016
@pnbecker
Copy link
Member

pnbecker commented Mar 9, 2016

Perhaps a silly question: but does a scrollabale iterator my have advantages in regards to ConncurrentModificationExceptions we have to fight from time to time?

@KevinVdV
Copy link
Member Author

KevinVdV commented Mar 9, 2016

@pnbecker I don't think so, you the "ConncurrentModificationExceptions" is due to the fact that all lists returned by hibernate data objects are persisted. So if you iterate over that list & then remove something from it you get the exception.
Because you are removing something in its entire while looping over the list it is part of.

This PR just optimizes the iterators returned by hibernate to use forward scrolling in the hopes that this is more memory efficient.

@tdonohue
Copy link
Member

tdonohue commented Mar 9, 2016

Since this PR seems to not be immediately beneficial, I'm going to close it for now. We can always reopen it later if we discover a benefit to the HibernateScrollableResultsIterator

@tdonohue tdonohue closed this Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants