1 2 Previous Next 19 Replies Latest reply on Jul 15, 2008 7:47 AM by twiceknightly

    does this design make sense?

    twiceknightly

      My app is set up as a stateless session bean calling jbpm. So within the enerprise bean I open the hibernate session, do my jbpm goodness and then save and close the hibernate session.

      I have a process definition with multiple tokens each signalling on different part of the process graph. The child tokens live for the life of the process i.e. there is no join.

      I was thinking that to stop classic concurrency problems like lost update problem I thought I might be able to use a special task for the purpose of locking i.e. use it as a lock. I could put something in the ejb, so before it performs the signal on one of the tokens (or whatever) , it has to acquire the lock (special task) for this process.

      Do you think this makes sense?

        • 1. Re: does this design make sense?
          kukeltje

          a special task, one special variable, one.... you name it.... The issue that you have to act on this special task in one way or another (when does concurrency end? what if this lock stays by accident, what do you return to the other token). Sound like your trying to invent a mechanism that maybe should be in the engine... I mean, maybe it has to be possible to set on each variable which token might update it/is working with it).... But you have to retrieve changed values in tasks then (or not use task scoped variables but only process scoped variables

          • 2. Re: does this design make sense?
            kukeltje

            maybe this discussion (the generic one on lost updates) should be held on the dev forum (again)

            • 3. Re: does this design make sense?
              doomi

              I'm facing the same problem: jBPM swallows the StaleObjectStateException causing the calling code not to notice that the operation failed.

              I figured out a workaround, but I'm not sure yet, whether it's proper or not. The idea is to do the hibernate-commit() by myself and so be able to receive the StaleObjectStateException in case there was a concurrent update on the process-instance. In this case a retry can be scheduled.

              ProcessInstance pi = jbpmContext.loadProcessInstanceForUpdate(id);
              
              pi.signal("someTransitionIfYouWant");
              
              boolean error = false;
              
               try {
               // commit() on hibernate-level
               jbpmContext.getSession().getTransaction().commit();
              
               } catch (StaleObjectStateException e) {
               error = true;
              
               } finally {
               // the tricky part: on the close()-method the jbpmContext tries to commit(), again!
               // to avoid irritating error-messages, we need to make the jbpmContext NOT try to commit()
               DbPersistenceService dbps =
               (DbPersistenceService) jbpmContext.getServices().getPersistenceService();
              
               // this can be achieved by pretending that transactions are disabled anyway
               dbps.setTransactionEnabled(false);
               jbpmContext.getServices().setPersistenceService(dbps);
              
               // close jbpmContext
               jbpmContext.close();
               }
              
              // handle error if present
              if(error){
              // e.g. schedule retry
              }
              else{
              ...
              }
              


              What do you guys think? Any suggestions?

              • 4. Re: does this design make sense?
                twiceknightly

                 

                "kukeltje" wrote:
                a special task, one special variable, one.... you name it.... The issue that you have to act on this special task in one way or another (when does concurrency end? what if this lock stays by accident, what do you return to the other token). Sound like your trying to invent a mechanism that maybe should be in the engine...


                Yes but the special task is shared. Two threads would race to acquire the special task (read lock) but ultimately a task is stored in the database so wouldn't the concurrency end when accessing a row in the task instance table? It seems to me as if there should be a mechanism in the engine to lock a process when there are multiple tokens.

                There may need to be a timer on the task to release it if it doesn't get released automatically or something.

                dOoMi. I would really appreciate it if you could write a paragraph on your understanding of the situation. I'm not deeply into Hibernate and have a really tight deadline for this stuff.

                To be fair I'm not familiar enough with jbpm yet to know at what level I need to provide synchronization. I presume it would be at the processInstance level.

                • 5. Re: does this design make sense?
                  doomi

                  as i see it, it works/fails like this:

                  1a. thread 1 opens a jbpmContext and a process instance
                  1b. thread 2 opens a jbpmContext and the same process instance
                  2a. thread 1 passes data to the process instance and gives a signal()
                  2b. thread 2 also passes data to the process instance and gives a signal()
                  3a. thread 1 processes the signal
                  3b. thread 2 processes the signal
                  ---now tatata, the problem occurs---
                  4a. thread 1 closes the jbpmContext, this causes the jbpm-engine to commit() the changed processInstance to the database -> OK
                  4.b thread 2 closes the jbpmContext, this causes the jbpm-engine to commit() the chanced processInstance to the database; commit() fails, because the processInstance on the database is different than it was when it was loaded; StaleObjectStateException is thrown AND CAUGHT by the jbpm-engine -> BAD, because the caller does not notice; data is lost, no retry can be scheduled

                  -> in my opinion the solution could be a method to tell, when optimistic locking failed to be able to retry. that's what i tried to do in my code.

                  • 6. Re: does this design make sense?
                    twiceknightly

                    dOoMi,

                    Where in the jbpm code is the StaleObjectStateException caught? Do you think my solution of using a process wide task as a lock would work?

                    kukeltje,

                    For us that are fairly new to jbpm and haven't followed the discussions in the past do you know where the discussion got to or be able to point us in the right direction regarding any resources?

                    cheers

                    • 7. Re: does this design make sense?
                      doomi

                      -> resources: i'm sorry, i'm new to the discussion board, too. so i have no idea where to find the original discussion thread.

                      -> exception: the exception is caught after all in org.jbpm.svc.Services in Line 226. The original Exception is caught earlier but gets wrapped into a JbpmPersistenceException and is thrown further.

                      -> locking-mechanism: you could also use a simple process variable to indicate the locking. as kukeltje stated the problem is the failover-scenario. what happens if the server crashes and the lock stays?

                      another approach might be to change the isolaotion-level of the database. this way you could take advantage of the existing locking-mechnism provided by your database.

                      • 8. Re: does this design make sense?
                        twiceknightly

                         

                        "dOoMi" wrote:
                        -> resources: i'm sorry, i'm new to the discussion board, too. so i have no idea where to find the original discussion thread.

                        -> exception: the exception is caught after all in org.jbpm.svc.Services in Line 226. The original Exception is caught earlier but gets wrapped into a JbpmPersistenceException and is thrown further.

                        -> locking-mechanism: you could also use a simple process variable to indicate the locking. as kukeltje stated the problem is the failover-scenario. what happens if the server crashes and the lock stays?

                        another approach might be to change the isolaotion-level of the database. this way you could take advantage of the existing locking-mechnism provided by your database.


                        It's good to have you on here dOoMi. Thanks for the info.

                        I didn't think you could use a process variable to do the locking because the contextInstance, in which the variables is stored, is also a shared object like the processInstance.

                        What I did think you could do is create a new task in your process instance that you have to claim and have in your personal task list before you can do anything. This is analogous to claiming a lock. Tasks can have timers so if the server crashes then the timer is still in the database and will eventually timeout. When it times out the task owner will be reset to null.

                        • 9. Re: does this design make sense?
                          doomi

                          Ah okay. I did not go deep into Tasks yet, cause it was not necessary in our workflows. The timout seems to be a good solution for the forever-locking-problem.

                          Can you please give me more details on your solution? I think i'm gonna try it, too.

                          cheers

                          • 10. Re: does this design make sense?
                            twiceknightly

                            Hi dOoMi,

                            I'm not at work so I can't remember the exact details of the calls etc however it would go something like that presented below. DISLAIMER: I'm not sure if this is enough to simulate a lock an prevent concurrency issues or not.

                            A task is meant to be a unit of work that the user performs via the UI. Generally tasks are assigned to a pool/group. So if an individual is a member of that pool/group then they can see that task. Tasks are then pulled or pushed from the pool to an individuals personal task list to prevent multiple people picking up the task. Once on the personal task list the user then completes the task. A fairly straight forward usage would be to have a task inside a task node and, by default I believe, when you signal the token into the task node it creates the task instance. When the token leaves the task node it closes the task. There are a lot of scenarios when using tasks but I believe that is probably the most common usage.

                            Ok so to use a task as a lock I would propose
                            1. In an action handler at the start of the process a call to
                            TaskMgmtInstance.createTaskInstance() to create a task solely to be used as a lock. (see section 11.2.2 of the user guide)
                            2. Create a new group in the database
                            3. Assign the task to the group via TaskInstance.setPooledActors()
                            4. Before executing an update of the process instance, e.g. a signal, you need to make a call to acquire the lock (i.e. you need to get the task assigned to you i.e. you pull it from the group list onto your task list). This is done by TaskInstance.setActorId().
                            5. Determine if you have the lock. Stated another way determine if the special task is on your personal work list with a call to TaskMgmtInstance.findTaskInstancesByActorId()
                            6. If you have the lock then you are free to carryout your actions on the process instance.
                            7. Free up the lock by doing TaskInstance.setActorId(null). This effectively takes the task out of your list and puts it back on the pooled/group list.

                            A timer may be needed to automatically free up the lock/task if not user(Section 13.1 of user manual).

                            It's very crude and I'm not even sure if it would work but I would love your thought on the idea.

                            • 11. Re: does this design make sense?
                              kukeltje

                               

                              jBPM swallows the StaleObjectStateException causing the calling code not to notice that the operation failed.
                              Afaik, that can be configured in jBPM 3.2.3. Please have a look in the docs

                              @Chessplayer....
                              I meant to start a new discussion on this 'lost updates' thing (detecting it, throwing errors, leaving as is...). The discussion was held years ago, so reviving is it is hard (old sourceforge forum). I'll draw something up in the weekend to start it.



                              • 12. Re: does this design make sense?
                                kukeltje

                                @rocade
                                Maybe it is better to realise this *outside* jBPM. It is good practice to store a little info as needed in the engine. Have a domain model outside jBPM and relate the data of a belonging to a processinstance to the processinstance by using the (business)key. This is a field on the processinstance.

                                That way, you can do locking or detecting changes (hibernate versioning!!) outside jBPM. I think that would be a better solution

                                • 13. Re: does this design make sense?
                                  twiceknightly

                                   

                                  "kukeltje" wrote:
                                  jBPM swallows the StaleObjectStateException causing the calling code not to notice that the operation failed.
                                  Afaik, that can be configured in jBPM 3.2.3. Please have a look in the docs


                                  kukeltje do you know roughtly where in the docs that is? I have taken a quick look and can't see it.

                                  cheers


                                  • 14. Re: does this design make sense?
                                    doomi

                                     

                                    "kukeltje" wrote:
                                    jBPM swallows the StaleObjectStateException causing the calling code not to notice that the operation failed.
                                    Afaik, that can be configured in jBPM 3.2.3. Please have a look in the docs


                                    i'm afraid not. i've been on this thing several hours. the configuration allows you to influence the logging but not the handling of StaleObjectStateExceptions. you can see that clearly when having a look at the source of the Services class:

                                    public void close() {
                                    236: if (services != null) {
                                    237: Map closeExceptions = new HashMap();
                                    238: Throwable firstException = null;
                                    239: Iterator iter = serviceNames.iterator();
                                    240: while (iter.hasNext()) {
                                    241: String serviceName = (String) iter.next();
                                    242: Service service = (Service) services.get(serviceName);
                                    243: if (service != null) {
                                    244: try {
                                    245: log.debug("closing service '" + serviceName
                                    246: + "': " + service);
                                    247: service.close();
                                    248: } catch (JbpmPersistenceException e) {
                                    249: // if this is a stale object exception, the jbpm configuration has control over the logging
                                    250: if ("org.hibernate.StaleObjectStateException"
                                    251: .equals(e.getCause().getClass()
                                    252: .getName())) {
                                    253: log.info("problem closing service '"
                                    254: + serviceName
                                    255: + "': optimistic locking failed");
                                    256: StaleObjectLogConfigurer.staleObjectExceptionsLog
                                    257: .error(
                                    258: "problem closing service '"
                                    259: + serviceName
                                    260: + "': optimistic locking failed",
                                    261: e);
                                    262: } else {
                                    263: log.error("problem closing service '"
                                    264: + serviceName + "'", e);
                                    265: }
                                    266: } catch (Exception e) {
                                    267: // NOTE that Error's are not caught because that might halt the JVM and mask the original Error.
                                    268: log.error("problem closing service '"
                                    269: + serviceName + "'", e);
                                    270: closeExceptions.put(serviceName, e);
                                    271: if (firstException == null) {
                                    272: firstException = e;
                                    273: }
                                    274: }
                                    275: }
                                    276: }
                                    277: if (!closeExceptions.isEmpty()) {
                                    278: throw new JbpmException("problem closing services "
                                    279: + closeExceptions, firstException);
                                    280: }
                                    281: }
                                    282: }
                                    283:
                                    284: public static void assignId(Object object) {
                                    285: JbpmContext jbpmContext = JbpmContext.getCurrentJbpmContext();
                                    286: if (jbpmContext != null) {
                                    287: // give this process instance an id
                                    288: Services services = jbpmContext.getServices();
                                    289: if (services.hasService(Services.SERVICENAME_PERSISTENCE)) {
                                    290: PersistenceService persistenceService = services
                                    291: .getPersistenceService();
                                    292: persistenceService.assignId(object);
                                    293: }
                                    294: }
                                    295: }


                                    1 2 Previous Next