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

Reuse joins for jurisdiction predicates [5] #8688

Closed
4 tasks done
Tracked by #7734
StefanKock opened this issue Apr 5, 2022 · 3 comments · Fixed by #8794 or #8859
Closed
4 tasks done
Tracked by #7734

Reuse joins for jurisdiction predicates [5] #8688

StefanKock opened this issue Apr 5, 2022 · 3 comments · Fixed by #8794 or #8859
Assignees
Labels
backend Affects the web backend change A change of an existing feature (ticket type) performance Issues that are meant to increase the performance of the application refactoring Technical refactoring of an existing feature

Comments

@StefanKock
Copy link
Contributor

StefanKock commented Apr 5, 2022

Problem Description

Coming from investigation in #8637: When calling "Entity"Service.inJurisdictionOrOwned from other contexts, there should be a reusage of joins to drastically reduce LEFT JOINS for example Task -> Contact -> Case

Proposed Change

  • 1. Do research where this pattern is causing too much joins. Known: Task, Sample
  • 2. Instantiate the QueryJoins object on the caller side (Task -> Case) and not on the called side.
  • 3. Reduce redundant getters in QueryJoins subclasses, link them to cached QueryJoins.
  • 4. Change all instantiation of QueryContexts with JPA Join outside it's domain for all queries that create jurisdiction Predicates.

Out of scope: Join reuse in createUserFilter methods -> #8747

Possible Alternatives

Additional Information

In the PoC I managed to get rid of 4 Case joins.

Before/After SQL comparison

Before:

-- TaskFacadeEjbTest.testFilterTasksByUserJurisdiction -> 2b. Region user now sees tasks from district level

LEFT OUTER JOIN cases case17_ ON contact11_.caze_id=case17_.id
LEFT OUTER JOIN Person person18_ ON case17_.person_id=person18_.id
LEFT OUTER JOIN cases case19_ ON contact11_.caze_id=case19_.id  --<- to be removed
LEFT OUTER JOIN cases case20_ ON contact11_.caze_id=case20_.id  --<- to be removed
LEFT OUTER JOIN cases case21_ ON contact11_.caze_id=case21_.id  --<- to be removed
LEFT OUTER JOIN cases case22_ ON contact11_.caze_id=case22_.id  --<- to be removed
LEFT OUTER JOIN Contact contacts23_ ON case22_.id=contacts23_.caze_id

After:

LEFT OUTER JOIN cases case15_ ON contact10_.caze_id=case15_.id
LEFT OUTER JOIN Person person16_ ON case15_.person_id=person16_.id
LEFT OUTER JOIN Contact contacts17_ ON case15_.id=contacts17_.caze_id

When I look at the needed changes, I get the impression that a massive overhaul is needed:

  1. Every creation of "Entity"QueryContext needs to be checked and all or most to be edited.
  2. Every "Entity"Joins object needs to be checked and delegation to lazily created Sub-"Entity"Joins are needed (removes a lot of duplicated code as seen in TaskJoins).
  3. The "Entity"Service.createUserFilter mostly needs to be called with a "Entity"Joins object instead of JPA Join as root object.
Helpful code snippets

To adapt the QueryContext subclasses:

// temporarly reduce to "protected", but needs to be changed back to "public" to not break createUserFilter (#8747)
	protected CaseQueryContext(CriteriaBuilder cb, CriteriaQuery<?> query, From<?, Case> root) {
		this(cb, query, new CaseJoins(root));
	}

	public CaseQueryContext(CriteriaBuilder cb, CriteriaQuery<?> query, CaseJoins joins) {
		super(cb, query, joins.getRoot(), joins);
	}

To lazily create cascaded QueryJoins:

	protected <T, A extends AbstractDomainObject, J extends QueryJoins<A>> J getOrCreate(J joins, Supplier<J> joinsSupplier, Consumer<J> setValue) {

		final J result;
		if (joins == null) {
			result = joinsSupplier.get();
			setValue.accept(result);
		} else {
			result = joins;
		}

		return result;
	}
@StefanKock StefanKock added backend Affects the web backend change A change of an existing feature (ticket type) performance Issues that are meant to increase the performance of the application labels Apr 5, 2022
@StefanKock StefanKock added this to Backlog in SORMAS Team 2 - DEV - Iteration Backlog via automation Apr 5, 2022
@StefanKock StefanKock self-assigned this Apr 5, 2022
@StefanKock StefanKock moved this from Backlog to Waiting in SORMAS Team 2 - DEV - Iteration Backlog Apr 5, 2022
@StefanKock StefanKock changed the title Reuse joins for jurisdiction checks Reuse joins for jurisdiction predicates and filters Apr 5, 2022
@StefanKock StefanKock changed the title Reuse joins for jurisdiction predicates and filters Reuse joins for jurisdiction predicates Apr 7, 2022
@StefanKock StefanKock assigned BarnaBartha and unassigned StefanKock Apr 7, 2022
@StefanKock
Copy link
Contributor Author

Waiting for #8748 to be merged.

StefanKock added a commit that referenced this issue Apr 8, 2022
Paths:
1. Task - Case - Person - Location
2. Task - Contact - Case - ...
@StefanKock
Copy link
Contributor Author

@BarnaBartha I started the work for this ticket on branch https://github.com/hzi-braunschweig/SORMAS-Project/tree/feature-8688_reuse_joins_for_jurisdiction_checks for you to pick it up.

@StefanKock StefanKock moved this from Waiting to Backlog in SORMAS Team 2 - DEV - Iteration Backlog Apr 8, 2022
@StefanKock StefanKock added the refactoring Technical refactoring of an existing feature label Apr 8, 2022
@BarnaBartha BarnaBartha moved this from Backlog to In Progress in SORMAS Team 2 - DEV - Iteration Backlog Apr 8, 2022
BarnaBartha added a commit that referenced this issue Apr 11, 2022
@vidi42 vidi42 changed the title Reuse joins for jurisdiction predicates Reuse joins for jurisdiction predicates [5] Apr 11, 2022
BarnaBartha added a commit that referenced this issue Apr 11, 2022
…text where root of query is not EventParticipant
BarnaBartha added a commit that referenced this issue Apr 11, 2022
…ry is not Event

      - clean up all remaining QueryJoins
BarnaBartha added a commit that referenced this issue Apr 12, 2022
@BarnaBartha
Copy link
Contributor

BarnaBartha commented Apr 19, 2022

Evaluation of performance improvement using national user:

Samples directory (546285 active entries / 819468 in total):

Screenshot 2022-04-19 at 10 59 28
Screenshot 2022-04-19 at 10 59 58

Tasks directory (30127 active entries / 92367 in total):

Screenshot 2022-04-19 at 10 57 29
Screenshot 2022-04-19 at 10 58 35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Affects the web backend change A change of an existing feature (ticket type) performance Issues that are meant to increase the performance of the application refactoring Technical refactoring of an existing feature
Projects
None yet
3 participants