-
Notifications
You must be signed in to change notification settings - Fork 355
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
fix: Only use occurreddate to filter events in exporter [DHIS2-17732] #19363
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you post some numbers on how that affects performance (and or explain analyze changes)?
...es/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java
Outdated
Show resolved
Hide resolved
In some implementations it passed from 95 seconds to 2 seconds, there is more info in the thread https://dhis2.atlassian.net/browse/DHIS2-17732?focusedCommentId=205282 The changes affected performance because before the |
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
…#19363) * fix: Only use occurreddate to filter events in exporter [DHIS2-17732] * Fix review comments
…#19363) * fix: Only use occurreddate to filter events in exporter [DHIS2-17732] * Fix review comments
…#19363) * fix: Only use occurreddate to filter events in exporter [DHIS2-17732] * Fix review comments
We were applying some magic between
occurreddate
andscheduleddate
in events and it was creating a performance issue and some odd behavior.Performance issue
occurreddate
, if there was a null value, then the one inscheduledate
was used. This behavior was preventing the index onoccurreddate
to be used by the query planner slowing down the query performance.Odd behavior
occurredAfter
andoccurredBefore
are used to filter theoccurreddate
but because of the logic that we had in the query, null values ofoccurreddate
could be part of the result.scheduleddate
withoccurreddate
value if the former one was null. This wrongly appears as if the event was once scheduled and occurred in the same point in time. If an event was never scheduled, it should have a nullscheduleddate
.These corner cases were not tested so no test is breaking. The feature itself is already properly tested in https://github.com/dhis2/dhis2-core/blob/master/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java