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

Hypersistence Optimizer Issues: Critical: EagerFetchingEvent #2220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ecruz165
Copy link
Contributor

Consider using lazy fetching which, not only that is more efficient, but is way more flexible when it comes to fetching data. For more info about this event, check out this User Guide link - https://vladmihalcea.com/hypersistence-optimizer/docs/user-guide/#EagerFetchingEvent

I left one reported below as is because it seem eager is necessary here to avoid no session error i encountered.
The [daimons] attribute in the [org.ohdsi.webapi.source.Source]
This article points out alternatives and i concluded keeping eager is right thing to do in this case.
https://www.baeldung.com/hibernate-initialize-proxy-exception

Reran report
BEFORE
165 issues were found: 1 BLOCKER, 80 CRITICAL, 40 MAJOR, 44 MINOR
AFTER
137 issues were found: 1 BLOCKER, 52 CRITICAL, 40 MAJOR, 44 MINOR

…hing which, not only that is more efficient, but is way more flexible when it comes to fetching data.
@chrisknoll
Copy link
Collaborator

chrisknoll commented Feb 24, 2023

Eager also prevents N+1 fetches which can be very bad for performance. I'm fairly certain where we use EAGER it is intentional and necessary, but I'll have to review the specific cases that you uncover.

Edit:
After reading the article, they describe one of the risks to EAGER is the N+1 problem, so I may be confusing techniques to avoid performance problems: What I was trying to remember was the use of EntityGraphs in order to fetch associated entities into a single request instead of the N+1 resut if you fetch the root and then sub-query the associated entities. So, we definitely need to review where EAGER is used, if it's resulting in performance issues and address accordingly.

@anthonysena anthonysena linked an issue Apr 4, 2023 that may be closed by this pull request
@anthonysena
Copy link
Collaborator

anthonysena commented Apr 4, 2023

Linking this to #2263

@anthonysena
Copy link
Collaborator

Associating with OHDSI/StandardizedAnalysisAPI#54

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