Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-12430

Fix excessive memory usage on requests with many action requests

    Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1.7
    • Fix Version/s: 2.4.1
    • Component/s: Performance
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-11793Rework collection paging to reduce memory usage
      KULRICE-7690Date Approved Workflow Preference can cause an excessive amount of sql queries
      KULRICE-6887RouteLog: Pending and Future Action Requests showing duplicate requests for the same account
      KULRICE-749Still seeing problems with orphaned Action Requests
      KULRICE-6848RouteLog: future action requests show invalid nodes when document is disapproved
      KULRICE-3337Allow for custom action request labels on ad hoc requests for more than just FYI type requests
      KULRICE-3603After a document is saved but before it is routed, the "Future Action Requests" tab on the doc's route log does not display any of the ad hoc requests
      KULRICE-7137Bug on Action List - Action Requested Label Blank after super user approve action
      KULRICE-3577WorkflowUtilityWebServiceImpl.getPrincipalIdsInRouteLog does not return principal ids for group members who have action requests "on the route log"
      KULRICE-7617The Super User Actions tag does not deal with action requests that are role requests
    • Rice Module:
      KEW
    • Application Requirement:
      KFS
    • Sprint:
      2.5.0-m2 Sprint 3
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      Documents with large number of action requests seem to be able to eat up all
      available memory on the KFS servers when performing document simulations.

      Apply the changes in the attached patch file to KEW and make the following change to OJB-repository-kew.xml

      in <class-descriptor class="org.kuali.rice.kew.actiontaken.ActionTakenValue" table="KREW_ACTN_TKN_T">

      Add proxy="true" to the collection descriptor for action requests as below:

      <collection-descriptor name="actionRequests" element-class-ref="org.kuali.rice.kew.actionrequest.ActionRequestValue"
      					   auto-retrieve="true" auto-update="false" auto-delete="false" proxy="true">
      	<inverse-foreignkey field-ref="actionTakenId" />
      </collection-descriptor>
      

      Summary of changes:

      1) Switch the action requests on action taken to lazy loading. (And adjust the getter so it uses a proxy-safe null check)
      2) In org.kuali.rice.kew.actionrequest.service.impl.ActionRequestServiceImpl.deactivateRequest(), comment out the actionTaken.getActionRequests().add(actionRequest); line to prevent lazy loading.

      From what I can tell, the only reason those requests on the action taken BO are ever used are to display on the route log and to send ACKs when you SU disapprove the document.

        Issue Links

          Activity

          Hide
          Jonathan Keller added a comment -

          Note - patch file is against KFS structure for customization. patch root will need to be adjusted to apply to Rice.

          Show
          Jonathan Keller added a comment - Note - patch file is against KFS structure for customization. patch root will need to be adjusted to apply to Rice.
          Hide
          Kristina Taylor (Inactive) added a comment -

          I've reviewed the patch. The first part seems to have been fixed, mostly because it is only run outside of a simulation, which seems to be the goal here:

          if (!activationContext.isSimulation()) {
            if (actionTaken != null) {
              // only add it if it's not null and we aren't in a simulation context, if we are in simulation mode, we
              // don't want to modify any action requests, lest they get saved by a JPA flush later!
              actionTaken.getActionRequests().add(actionRequest);
            }
            actionRequest = getDataObjectService().save(actionRequest);
            deleteActionItems(actionRequest, true);
          }
          

          The second part is ok, but the methods should be taken out completely, not commented out. As for the OJB file, the equivalent JPA mapping lazy loads by default, so I don't think anything needs to be done here. Can you verify?

          Show
          Kristina Taylor (Inactive) added a comment - I've reviewed the patch. The first part seems to have been fixed, mostly because it is only run outside of a simulation, which seems to be the goal here: if (!activationContext.isSimulation()) { if (actionTaken != null ) { // only add it if it's not null and we aren't in a simulation context, if we are in simulation mode, we // don't want to modify any action requests, lest they get saved by a JPA flush later! actionTaken.getActionRequests().add(actionRequest); } actionRequest = getDataObjectService().save(actionRequest); deleteActionItems(actionRequest, true ); } The second part is ok, but the methods should be taken out completely, not commented out. As for the OJB file, the equivalent JPA mapping lazy loads by default, so I don't think anything needs to be done here. Can you verify?
          Hide
          Kristina Taylor (Inactive) added a comment -

          Jonathan Keller: looking now
          [9:15:44 AM] Jonathan Keller: Not sure if that will help. We don't need to update that list even on non-simulations
          [9:15:58 AM] Jonathan Keller: I mean - it will help during simulations - but not during routing
          [9:17:02 AM] Jonathan Keller: And yes, the methods should be removed. The commenting is what I do when modifying baseline code at UCD - since I want the changes I made to be evident when looking at the files.
          [9:17:58 AM] Jonathan Keller: From my research - you should be able to remove the:

          if (actionTaken != null)

          { // only add it if it's not null and we aren't in a simulation context, if we are in simulation mode, we // don't want to modify any action requests, lest they get saved by a JPA flush later! actionTaken.getActionRequests().add(actionRequest); }

          section completely
          [9:18:29 AM] Jonathan Keller: otherwise the actionTaken.getActionRequests().add() causes a load of all the child requests
          [9:18:32 AM] Kristina B. Taylor: Ok, cool, because it looked like the thing was updated for JPA so I thought it might be safe now, but I wanted to check.
          [9:18:40 AM] Jonathan Keller: It's probably better
          [9:18:55 AM] Kristina B. Taylor: So if we leave the JPA mapping as it is now is that going to be ok?
          [9:18:55 AM] Jonathan Keller: we did run into the memory issue partly when the KFS purchasing module ran lots of simulations
          [9:19:04 AM] Jonathan Keller: Yes the JPA mapping is OK
          [9:19:17 AM] Kristina B. Taylor: Ok, so just remove those two pieces of code and we should be set then?
          [9:19:20 AM] Jonathan Keller: yep
          [9:19:26 AM] Kristina B. Taylor: Sweet, thanks!
          [9:19:32 AM] Jonathan Keller: np

          Show
          Kristina Taylor (Inactive) added a comment - Jonathan Keller: looking now [9:15:44 AM] Jonathan Keller: Not sure if that will help. We don't need to update that list even on non-simulations [9:15:58 AM] Jonathan Keller: I mean - it will help during simulations - but not during routing [9:17:02 AM] Jonathan Keller: And yes, the methods should be removed. The commenting is what I do when modifying baseline code at UCD - since I want the changes I made to be evident when looking at the files. [9:17:58 AM] Jonathan Keller: From my research - you should be able to remove the: if (actionTaken != null) { // only add it if it's not null and we aren't in a simulation context, if we are in simulation mode, we // don't want to modify any action requests, lest they get saved by a JPA flush later! actionTaken.getActionRequests().add(actionRequest); } section completely [9:18:29 AM] Jonathan Keller: otherwise the actionTaken.getActionRequests().add() causes a load of all the child requests [9:18:32 AM] Kristina B. Taylor: Ok, cool, because it looked like the thing was updated for JPA so I thought it might be safe now, but I wanted to check. [9:18:40 AM] Jonathan Keller: It's probably better [9:18:55 AM] Kristina B. Taylor: So if we leave the JPA mapping as it is now is that going to be ok? [9:18:55 AM] Jonathan Keller: we did run into the memory issue partly when the KFS purchasing module ran lots of simulations [9:19:04 AM] Jonathan Keller: Yes the JPA mapping is OK [9:19:17 AM] Kristina B. Taylor: Ok, so just remove those two pieces of code and we should be set then? [9:19:20 AM] Jonathan Keller: yep [9:19:26 AM] Kristina B. Taylor: Sweet, thanks! [9:19:32 AM] Jonathan Keller: np

            People

            • Assignee:
              Kristina Taylor (Inactive)
              Reporter:
              Jonathan Keller
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 4 hours
                4h
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 2 hours Time Not Required
                2h

                  Agile

                    Structure Helper Panel