Uploaded image for project: 'Kuali Rice Development'
  1. Kuali Rice Development
  2. KULRICE-12430

Fix excessive memory usage on requests with many action requests

    Details

    • Type: Task
    • Status: Closed
    • Priority: 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
    • 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.

        Attachments

          Issue Links

            Activity

            Hide
            jkeller 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
            jkeller 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
            kbtaylor 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
            kbtaylor 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
            kbtaylor 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
            kbtaylor 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:
                kbtaylor Kristina Taylor (Inactive)
                Reporter:
                jkeller 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