1

I have a method in CDI bean which is transactional, on error it creates an entry in database with the exception message. This method can be called by RESTendpoint and in multithread way.

We have a SQL constraint to avoid duplicity in database

    @Transactional
public RegistrationRuleStatus performCheck(RegistrationRule rule, User user) {

    try {
        //check if rule is dependent of other rules and if all proved, perform check
        List<RegistrationRule> rules = rule.getRuleParentDependencies();
        boolean parentDependenciesAreProved = true;

        if (!CollectionUtils.isEmpty(rules)) {
            parentDependenciesAreProved = ruleDao.areParentDependenciesProved(rule,user.getId());
        }

        if (parentDependenciesAreProved) {
            Object service = CDI.current().select(Object.class, new NamedAnnotation(rule.getProvider().name())).get();
            Method method = service.getClass().getMethod(rule.getProviderType().getMethod(), Long.class, RegistrationRule.class);

            return (RegistrationRuleStatus) method.invoke(service, user.getId(), rule);

        } else {
            RegistrationRuleStatus status = statusDao.getStatusByUserAndRule(user, rule);
            if (status == null) {
                status = new RegistrationRuleStatus(user, rule, RegistrationActionStatus.START, new Date());
                statusDao.create(status);
            }

            return status;
        }
    } catch (Exception e) {
        LOGGER.error("could not perform check {} for provider {}", rule.getProviderType().name(), rule.getProvider().name(), e.getCause()!=null?e.getCause():e);

        return statusDao.createErrorStatus(user,rule,e.getCause()!=null?e.getCause().getMessage():e.getMessage());
    }
}

create Error method:

@Transactional
public RegistrationRuleStatus createErrorStatus(User user, RegistrationRule rule, String message) {
     RegistrationRuleStatus status = getStatusByUserAndRule(user, rule);
     if (status == null) {
         status = new RegistrationRuleStatus(user, rule, RegistrationActionStatus.ERROR, new Date());
         status.setErrorCode(CommonPropertyResolver.getMicroServiceErrorCode());
         status.setErrorMessage(message);
         create(status);
     }else {
         status.setStatus(RegistrationActionStatus.ERROR);
         status.setStatusDate(new Date());
         status.setErrorCode(CommonPropertyResolver.getMicroServiceErrorCode());
         status.setErrorMessage(message);
         update(status);
     }
     return status;
}

the problem is method is called twice at same time and the error recorded is DuplicateException but we don't want it. We verify at the beginning if object already exists, but I think it is called at exactly same time.

JAVA8/wildfly/CDI/JPA/eclipselink

Any idea?

Jason Aller
  • 3,541
  • 28
  • 38
  • 38
cyril
  • 872
  • 6
  • 29

1 Answers1

1

I'd suggest you to consider following approaches:

1) Implement retry logic. Catch exception, analyze it. If it indicates an unexpected duplicate (like you described), then don't consider it as an error and just repeat the method call. Now your code will work differently: It will notice that a record already exists and will not create a duplicate.

2) Use isolation level SERIALIZABLE. Then within a single transaction your will "see" a consistent behaviour: If select operation hasn't found a particular record, then till the end of this transaction no other transaction will insert such record and there will be no exception related to duplicates. But the price is that the whole table will be locked for each such transaction. This can degrade the application performance essentially.

mentallurg
  • 4,967
  • 5
  • 28
  • 36
  • Thank you , second is not possible because we are in microservicies and using same database. What i do no understand is we have @Transactional, it means we are in a atomic operation, how it is possible to enter in the method twice at same time, because in this code we verify if entry already exists! – cyril Oct 23 '19 at 06:10
  • I'm afraid your understanding of *atomic* is not quit correct. Atomic means that if you do multiple changes in database, either they all will be applied or none of them will be applied. Suppose you change records in 3 tables. Updates in 2 tables were successful, but update in the 3rd table caused a constraint violation. Atomic means that in such case changes in all 3 tables will be rolled back to the original state. – mentallurg Oct 23 '19 at 19:44
  • Annotation *@transactional* does not mean that the method is not allowed to be called from multiple threads simultaneously. It can work as follows: Transaction A sees that there is no record. A bit later transaction B sees that there is no record. A bit later transaction A inserts record and completes successfully. A bit later transaction B attempts to insert record, but an exception occurs because there is already a record created by transaction B. This is absolutely normal and correct behaviour of the application and of the database. You can handle it by solution 1) or 2). – mentallurg Oct 23 '19 at 20:07
  • The expectation that at any time only one transaction can modify data is not correct. This behaviour is controlled by isolation level. There is a lot of documentation about this, just google for it. Briefly, all transactions are basically allowed to be executed in parallel. If you want to be sure that when this transaction is running no other transaction changes relevant data, you should use isolation level SERIALIZABLE. Then all the transactions that attempt to modify the same data will be blocked until your transaction is completed. But the price is performance. – mentallurg Oct 23 '19 at 20:10
  • @cyril: If the answer is helpful, you can mark it as helpful. – mentallurg Oct 24 '19 at 22:09