5

i m in trouble with a simple REST service using this code :

@GET
@Path("next/{uuid}")
@Produces({"application/xml", "application/json"})
public synchronized Links nextLink(@PathParam("uuid") String uuid) {
    Links link = null;
    try {
        link = super.next();
        if (link != null) {
            link.setStatusCode(5);
            link.setProcessUUID(uuid);
            getEntityManager().flush(); 
            Logger.getLogger("Glassfish Rest Service").log(Level.INFO, "Process {0} request url : {1} #id  {2} at {3} #", new Object[]{uuid, link.getLinkTxt(), link.getLinkID(), Calendar.getInstance().getTimeInMillis()});
        }
    } catch (NoResultException ex) {
    } catch (IllegalArgumentException ex) {
    }
    return link;
}

this should provide a link object, and mark it as used (setStatusCode(5)) to prevent next access to service to send the same object. the probleme, is that when there s a lot of fast clients accessing to the web service, this one provides 2 or 3 times the same link object to different clients. how can i solve this ??

here is the resquest using to : @NamedQuery(name = "Links.getNext", query = "SELECT l FROM Links l WHERE l.statusCode = 2")

and the super.next() methode :

public T next() {

    javax.persistence.Query q = getEntityManager().createNamedQuery("Links.getNext");
    q.setMaxResults(1);
    T res = (T) q.getSingleResult();
    return res;
}

thx

2
  • 4
    Not sure I can answer a question in which you swallow exceptions! Feb 4, 2013 at 10:34
  • not sure this kind of answer can help anyone here... Feb 4, 2013 at 17:28

4 Answers 4

7

The life-cycle of a (root) JAX-RS resource is per request, so the (otherwise correct) synchronized keyword on the nextLink method is sadly ineffectual.

What you need is a mean to synchronize the access/update. This could be done in many ways:

I) You could synchronize on an external object, injected by a framework (example: a CDI injected @ApplicationScoped) as in:

@ApplicationScoped
public class SyncLink{
    private ReentrantLock lock = new ReentrantLock();
    public Lock getLock(){
       return lock;
    }
}
....
public class MyResource{
  @Inject SyncLink sync;

  @GET
  @Path("next/{uuid}")
  @Produces({"application/xml", "application/json"})
  public Links nextLink(@PathParam("uuid") String uuid) {
    sync.getLock().lock();
    try{
      Links link = null;
      try {
        link = super.next();
        if (link != null) {
            link.setStatusCode(5);
            link.setProcessUUID(uuid);
            getEntityManager().flush(); 
            Logger.getLogger("Glassfish Rest Service").log(Level.INFO, "Process {0} request url : {1} #id  {2} at {3} #", new Object[]{uuid, link.getLinkTxt(), link.getLinkID(), Calendar.getInstance().getTimeInMillis()});
        }
      } catch (NoResultException ex) {
      } catch (IllegalArgumentException ex) {
      }
      return link;
    }finally{
       sync.getLock().unlock();
    }
  }
}

II) You could be lazy and synchronize on the class

public class MyResource{
  @Inject SyncLink sync;

  @GET
  @Path("next/{uuid}")
  @Produces({"application/xml", "application/json"})
  public Links nextLink(@PathParam("uuid") String uuid) {
     Links link = null;
    synchronized(MyResource.class){
      try {
        link = super.next();
        if (link != null) {
            link.setStatusCode(5);
            link.setProcessUUID(uuid);
            getEntityManager().flush(); 
            Logger.getLogger("Glassfish Rest Service").log(Level.INFO, "Process {0} request url : {1} #id  {2} at {3} #", new Object[]{uuid, link.getLinkTxt(), link.getLinkID(), Calendar.getInstance().getTimeInMillis()});
        }
      } catch (NoResultException ex) {
      } catch (IllegalArgumentException ex) {
      }

    }
    return link;
  }
}

III) You could synchronize using the database. In that case you would investigate the pessimistic locking available in JPA2.

6
  • I tryed solution I. i have now less 'collisions', i.e less links provided many times to clients, but the lock is not 100% efficient Feb 6, 2013 at 10:08
  • Are you deploying on more than one application server? BTW: which application server do you use? Feb 6, 2013 at 10:14
  • I m using glassfish 3.1.2 for the devel. the application is deployed only on one server and only one instance of the application is running. the problem occurs only at start up, when all the clients connect to the glassfish rest service and request links. Running 6 simultaneous clients is enough to observe the bad behaviour of the REST service Feb 6, 2013 at 10:51
  • All right, my guess is that the JPA transaction gets committed after the guarded block. Have you anything against a complete refactoring of the next() and update code into a @Singleton session bean? It could solve the problem (perhaps better/more reliable) way. If so, I'll update the answer to reflect it Feb 6, 2013 at 11:41
  • You are probably right as the 'collisions' only occurs when delta time between 2 requests is lower than 60 ms. I m now already doing what you have suggested. Hope it will fix my problem. Feb 6, 2013 at 12:50
0

You need to use some form of locking, most likely optimistic version locking. This will ensure only one transaction succeeds, the other will fail.

See, http://en.wikibooks.org/wiki/Java_Persistence/Locking

2
  • thx a lot, it seems to be 'theoretically' the good solution. i still testing all locks and hope i can soon have a good solution Feb 4, 2013 at 17:27
  • when i try to deal with JPA2 locks, i have many org.eclipse.persistence.exceptions.* (* depends of the locktype i try to use) that i can not catch or dont know where to catch Feb 6, 2013 at 10:19
0

Depending on how frequent you believe the contention will be in creating new Links you should choose either Optimistic locking using a @Version property or Pessimistic locking.

My guess is optimistic locking will work out better for you. In any case let your Resource class act as a Service Facade and place the model related code into a Stateless Session Bean EJB and handle any OptimisticLockExceptions with a simply retry.

I noticed you mentioned you are having trouble catching locking related exceptions and it also looks like you are using Eclipselink. In that case you could try something like this:

@Stateless
public class LinksBean {

  @PersistenceContext(unitName = "MY_JTA_PU")
  private EntityManager em;

  @Resource
  private SessionContext sctx;

  public Links createUniqueLink(String uuid) {
    Links myLink = null;
    shouldRetry = false;
    do {
      try
        myLink = sctx.getBusinessObject(LinksBean.class).createUniqueLinkInNewTX(uuid);
      }catch(OptimisticLockException olex) {
        //Retry
        shouldRetry = true;
      }catch(Exception ex) {
       //Something else bad happened so maybe we don't want to retry 
       log.error("Something bad happened", ex);
       shouldRetry = false;   
    } while(shouldRetry);
    return myLink;
  }

  @TransactionAttribute(TransactionAttributeType.REQUIRES_NEW)
  public Links createUniqueLinkInNewTX(uuid) {
      TypedQuery<Links> q = em.createNamedQuery("Links.getNext", Links.class);
      q.setMaxResults(1);
      try {
        myLink = q.getSingleResult();
      }catch(NoResultException) {
        //No more Links that match my criteria
        myLink = null;
      }
      if (myLink != null) {
        myLink.setProcessUUID(uuid);
        //If you change your getNext NamedQuery to add 'AND l.uuid IS NULL' you 
        //could probably obviate the need for changing the status code to 5 but if you 
        //really need the status code in addition to the UUID then:
        myLink.setStatusCode(5);
      }
      //When this method returns the transaction will automatically be committed 
      //by the container and the entitymanager will flush. This is the point that any 
      //optimistic lock exception will be thrown by the container. Additionally you 
      //don't need an explicit merge because myLink is managed as the result of the 
      //getSingleResult() call and as such simply using its setters will be enough for 
      //eclipselink to automatically merge it back when it commits the TX
      return myLink; 
  }  
}

Your JAX-RS/Jersey Resource class should then look like so:

@Path("/links")
@RequestScoped
public class MyResource{
  @EJB LinkBean linkBean;

  @GET
  @Path("/next/{uuid}")
  @Produces({"application/xml", "application/json"})
  public Links nextLink(@PathParam("uuid") String uuid) {
     Links link = null;
     if (uuid != null) {
         link = linkBean.createUniqueLink(uuid);
         Logger.getLogger("Glassfish Rest Service").log(Level.INFO, "Process {0} request url : {1} #id  {2} at {3} #", new Object[]{uuid, link.getLinkTxt(), link.getLinkID(), Calendar.getInstance().getTimeInMillis()});
      }
    return link;
  }
}

That's a semi-polished example of one approach to skin this cat and there's a lot going on here. Let me know if you have any questions.

Also, from the REST end of things you might consider using @PUT for this resource instead of @GET because your endpoint has the side effect of updating (UUID and/or statusCode) the state of the resource not simply fetching it.

0

When using JAX-RS which is a Java EE feature, in my understanding you should not manage threads in Java SE style like using synchronized block.

In Java EE you can provide synchronized access to your method with singleton EJB:

@Path("")
@Singleton
public class LinksResource {

    @GET
    @Path("next/{uuid}")
    @Produces({"application/xml", "application/json"})
    public Links nextLink(@PathParam("uuid") String uuid) {

By default this will use @Lock(WRITE) which allows only one request at a time to your method.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.