Skip to content
This repository has been archived by the owner on May 18, 2020. It is now read-only.

fix(AAE-1207): Updated graphql-jpa-query version to 0.4.0 #234

Merged
merged 4 commits into from
Jan 23, 2020

Conversation

igdianov
Copy link
Contributor

@igdianov igdianov commented Jan 21, 2020

This PR updates graphql-jpa-query dependencies in order to fix GraphQL JPA query memory leaks caused by JPA provider paginated collection fetch:

HHH000104: firstResult/maxResults specified with collection fetch; applying in memory!

It also removes redundant implementations of GraphQLController and GraphQLSchema auto configuration managed by graphql-jpa-query-web and graphql-jpa-query-autoconfigure modules.

I have built the preview and tested the fix for Notification GraphQL Query running out of memory problem with Activiti Cloud Helm deployment in K8s against 130,000 tasks in the Postgresql Query database: AlfrescoArchive/activiti-cloud-notifications-graphql#314

The Gatling load test simulation was running task query below with collection fetches for 100 random pages with 100 limit by ramping concurrent users from 1 to 100 during 3 minutes time period:

query find($page: Int!, $limit: Int!) {
  Tasks(
    where: {taskCandidateGroups: {groupId: {IN: "hr"}}, status: {EQ: CREATED}}, 
    page: {start: $page, limit: $limit}
  ) {
    select {
      id
      name
      status
      variables {
        id
        name
        value
        type
      }
      users: taskCandidateUsers {
        userId
      }
      groups: taskCandidateGroups {
        groupId
      }
      processInstance {
        id
        status
        processDefinitionKey
        variables {
          id
          name
          value
          type
        }
      }
    }
  }
}

There is a total of 5511 requests reported 100% successful response, with 95th percentile having 1464ms response time (i.e. 5% of requests had response time more than 1.4 seconds)

image

image

Fixes AAE-1207

@igdianov igdianov added the enhancement New feature or request label Jan 21, 2020
@igdianov igdianov self-assigned this Jan 21, 2020
@codecov
Copy link

codecov bot commented Jan 21, 2020

Codecov Report

Merging #234 into develop will decrease coverage by 1.79%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             develop    #234     +/-   ##
===========================================
- Coverage      65.59%   63.8%   -1.8%     
+ Complexity       207     188     -19     
===========================================
  Files             48      44      -4     
  Lines           1180    1105     -75     
  Branches          87      82      -5     
===========================================
- Hits             774     705     -69     
+ Misses           355     353      -2     
+ Partials          51      47      -4
Impacted Files Coverage Δ Complexity Δ
.../query/ActivitiGraphQLSchemaAutoConfiguration.java 100% <ø> (ø) 1 <0> (ø) ⬇️
...utoconfigure/ActivitiGraphQLAutoConfiguration.java 100% <ø> (ø) 1 <0> (ø) ⬇️
.../config/GraphQLSubscriptionsAutoConfiguration.java 100% <ø> (ø) 1 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77fb313...2cce1d9. Read the comment docs.

@igdianov igdianov force-pushed the fix-AAE-1207-update-graphql-jpa-query-version branch from cd1febe to 2cce1d9 Compare January 23, 2020 07:19
@claassistantio
Copy link

claassistantio commented Jan 23, 2020

CLA assistant check
All committers have signed the CLA.

@igdianov igdianov removed the request for review from erdemedeiros January 23, 2020 16:43
@mergify mergify bot merged commit 664eae7 into develop Jan 23, 2020
@mergify mergify bot deleted the fix-AAE-1207-update-graphql-jpa-query-version branch January 23, 2020 17:56
@erdemedeiros
Copy link
Contributor

Ref Activiti/Activiti#3139

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants