29

What is the correct pattern for handling OLE in a (REST) web service? this is what I'm doing now, for example,

protected void doDelete(HttpServletRequest request, HttpServletResponse response)
        throws ServletException, IOException {

    ...
    ...
    ...

    try {
        try {
            em.getTransaction().begin();
            // ... remove the entity
            em.getTransaction().commit();
        } catch (RollbackException e) {
            if (e.getCause() instanceof OptimisticLockException) {
                try {
                    CLog.e("optimistic lock exception, waiting to retry ...");
                    Thread.sleep(1000);
                } catch (InterruptedException ex) {
                }
                doDelete(request, response);
                return;
            }
        }

        // ... write response

    } catch (NoResultException e) {
        response.sendError(HttpServletResponse.SC_NOT_FOUND, e.getMessage());
        return;
    } finally {
        em.close();
    }
}

anytime you see a sleep in the code, there's a good chance it's incorrect. Is there a better way to handle this?

another approach would be to immediately send the failure back to the client, but I'd rather not have them worry about it. the correct thing seems to do whatever is required to make the request succeed on the server, even if it takes a while.

Nimantha
  • 6,405
  • 6
  • 28
  • 69
Jeffrey Blattman
  • 22,176
  • 9
  • 79
  • 134

4 Answers4

35

If you get an optimistic locking exception, it means that some other transaction has committed changes to entities you were trying to update/delete. Since the other transaction has committed, retrying immediately might have a good chance to succeed.

I would also make the method fail after N attempts, rather than waiting for a StackOverflowException to happen.

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • thanks, this is what i ended up implementing. in my case, there is a very good chance of succeeding on the second try. that being said, i acknowledge that the correct thing to do as suggested by Augusto is to let the client re-try. – Jeffrey Blattman Jul 03 '11 at 18:34
  • 1
    Another reason why you can get an OLE is because the entity [version](http://docs.oracle.com/javaee/6/api/javax/persistence/Version.html) differs from the one that's on the db. It happens on update operations and retrying after sometime won't help. Also notice that the entity you are going to update must have getter and setter methods for the version. It sounds logical, but for instance, I was doing some type of Generic repository for handling basic CRUD operations and JPA was not able to access the version's setter giving an OLE on the next update. – Jonathan Morales Vélez Feb 17 '14 at 16:26
  • 1
    Just 'retrying immediately' is not a good idea at all. This exception happens because 2 users are editing the same piece of information at the same time. One will succeed, while the other one would simply overwrite what the first one did. Optimistic locking will avoid that by throwing an exception on the second commit. This requires user intervention first to make sure the two versions of input get verified and merged one way or the other. – YoYo Aug 24 '17 at 20:03
  • Automatically retrying an operation on `OptimisticLockException` is not a good idea, because a significant update could be lost then. – deamon Jan 18 '22 at 07:45
31

The "politically correct" answer in rest, is to return an HTTP 409 (Conflict) witch matches perfectly with the idea of optimistic locking. Your client should manage it, probably by retring a few seconds later.

I wouldn't add the logic to retry in your app, as your client will already handle situations when you return a 40X code.

Augusto
  • 28,839
  • 5
  • 58
  • 88
  • Not a useful return if you're in a standalone application. It's only meaningful if you happen to be inside a web oriented environment. – Evvo Jan 23 '20 at 17:45
1

By the way, catch (InterruptedException e) {} is always a bad idea, because the system has asked your computation to cancel, and you are ignoring it. In the context of a web service, an InterruptedException would be another good reason to signal an error to the client.

Unai Vivi
  • 3,073
  • 3
  • 30
  • 46
Jason Penney
  • 19
  • 1
  • 1
    Actually, the system is forcing the code to stop whether he logs the exception or not. After the exception is caught, execution does not return to the try block. – le3th4x0rbot Sep 13 '13 at 15:08
  • 1
    While this is true it does not answer the question. I also want to add that catching the exception is fine. You should however then call `Thread.currentThread.interrupt()` because the thread's interrupt status was cleared by the exception. – Björn Zurmaar Jun 19 '20 at 08:08
0

If you're just going to keep retrying until it works anyway, why not just disable optimistic locking? You should let the caller know that they made a decision based on out dated information! If you're in control of both sides an appropriate 400 code can be returned. If it's public it can be more friendly to arbitrary clients to just return 500. (Of course then you perpetuate the under-use of appropriate response codes! such a dilemma)

Affe
  • 47,174
  • 11
  • 83
  • 83