0

I'm trying to write a default method in a CrudRepository dao that:

  1. Locks a table such that inserts are prevented (but reads and updates would be okay)
  2. Looks for an existing entry using another method in the dao.
    • If found, returns the found entry.
    • If not found, inserts the provided entry, and returns it.
  3. Unlocks the table.

I've looked into using @Lock(LockModeType.PESSIMISTIC_WRITE) on the method, but since it doesn't have an associated @Query annotation, I don't think it does anything.

I also tried creating lock and unlock methods in the dao:

    @Query(value = "LOCK TABLES mail WRITE", nativeQuery = true)
    void lockTableForWriting();

    @Query(value = "UNLOCK TABLES", nativeQuery = true)
    void unlockTable();

But those threw a SQLGrammerException on LOCK and UNLOCK.

I can't get a row lock because the row doesn't exist yet. Or if it does exist, I'm not going to update anything, I'm just going to not insert anything and move on to other things.

There are other cases where I want multiple records saved with the same transaction id, so I can't just make the column unique and try/catch the save.

In my service layer, I do some lookup attempts and short-circuit when possible, but there's still a chance that multiple near-simultaneous calls might result in two attempts to insert the same data.

It needs to be handled at the db level since there will be multiple instances of the service running. So two competing calls might be on different machines that talk to the same database.

Here's my repository:

@Repository
public interface MailDao extends CrudRepository<Mail, Long> {

    default Mail safeSave(Mail mail) {
        return Optional.ofNullable(findByTransactionId(mail.getTransactionId()))
                       .orElseGet(() -> save(mail));
    }

    default Mail findByTransactionId(String transactionId) {
        final List<Mail> mails = findAllByTransactionId(transactionId);
        // Snipped code that selects a single entry to return
        // If one can't be found, null is returned.
    }

    @Query(value = "SELECT m " +
                   "  FROM Mail m " +
                   " WHERE m.transactionId = :transactionId ")
    List<Mail> findAllByTransactionId(@Param("transactionId") String transactionId);
}

And here's a bit of what the Mail model looks like:

@Entity
@Table(name = "mail")
public class Mail implements Serializable {
    private static final long serialVersionUID = 1L;

    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(name = "mail_id", unique = true, nullable = false)
    private Long mailId;

    @Column(name = "transaction_id", nullable = false)
    private String transactionId;

    // Snipped code for other parameters, 
    // constructors, getters and setters.
}

Here's the general idea of the service method that would be calling safeSave.

@Service
public class MailServiceImpl implements MailService {
    @Inject private final MailDao mailDao;
    // Snipped injected other stuff

    @Override
    public void saveMailInfo(final String transactionId) {
        Objects.requireNonNull(transactionId, "null transactionId passed to saveMailInfo");
        if (mailDao.findByTransactionId(transactionId) != null) {
            return;
        }
        // Use one of the injected things to do some lookup stuff
        // using external services
        if (mailDao.findByTransactionId(transactionId) != null) {
            return;
        }
        // Use another one of the injected things to do more lookup
        if (/* we've got everything we need */) {
            final Mail mail = new Mail();
            mail.setTransactionId(transactionId);
            // Snipped code to set the rest of the stuff
            mailDao.safeSave(mail);
        }
    }
}

What I'm trying to prevent is two nearly-simultaneous calls to saveMailInfo causing duplicate records in the database.

The underlying database is MySQL, but we also use an H2 in-memory database for unit tests, and will be changing to PostgreSQL soon, so it'd be nice to have something db-agnostic.

Update 1: I tried creating a custom impl:

public interface MailDaoCustom {
    Mail safeSave(Mail mail);
}

Updated MailDao to implement it:

public interface MailDao extends CrudRepository<Mail, Long>, MailDaoCustom

Then the impl:

public class MailDaoImpl implements MailDaoCustom {

    @Autowired private MailDao mailDao;
    @Autowired private EntityManager em;

    public Mail safeSave(Mail mail) {
        Query lockTable = em.createNativeQuery("LOCK TABLES mail WRITE");
        Query unlockTable = em.createNativeQuery("UNLOCK TABLES");
        try {
            lockTable.executeUpdate();
            return Optional.ofNullable(mailDao.findByTransactionId(mail.getTransactionId()))
                           .orElseGet(() -> mailDao.save(mail));
        } finally {
            unlockTable.executeUpdate();
        }
    }
}

This is the error I got when testing the above. It's the same as when I tried making the methods in the dao with the queries to lock and unlock:

SQL Error: 42000, SQLState: 42000
Syntax error in SQL statement "LOCK[*] TABLES MAIL WRITE "; SQL statement:
LOCK TABLES mail WRITE [42000-168]

My testing has been done using unit tests that use an H2 in-memory database, though, and not MySQL. It looks like H2 might not really have table locking, though.

Is there maybe another solution to my issue that doesn't involve table locking?

Update 2: I went with an INSERT ... WHERE NOT EXISTS query in a custom repo similar to my first update:

public interface MailDaoCustom {
    Mail safeSave(Mail mail);
}

Updated MailDao to implement it:

public interface MailDao extends CrudRepository<Mail, Long>, MailDaoCustom

And then the impl looks like this:

public class MailDaoImpl implements MailDaoCustom {

    @Autowired private MailDao dao;
    @Autowired private EntityManager em;

    public Mail safeSave(Mail mail) {
        // Store a new mail record only if one doesn't already exist.
        Query uniqueInsert = em.createNativeQuery(
                        "INSERT INTO mail " +
                        "       (transaction_id, ...) " +
                        "SELECT :transactionId, ... " +
                        " WHERE NOT EXISTS (SELECT 1 FROM mail " +
                        "                   WHERE transaction_id = :transactionId) ");
        uniqueInsert.setParameter("transactionId", mail.getTransactionId());
        // Snipped setting of the rest of the parameters in the query
        uniqueInsert.executeUpdate();

        // Now go get the record
        Mail entry = dao.findByTransactionId(mail.getTransactionId());
        // Detatch the entry so that we can attach the provided mail object later.
        em.detach(entry);

        // Copy all the data from the db entry into the one provided to this method
        mail.setMailId(entry.getMailId());
        mail.setTransactionId(entry.getTransactionId());
        // Snipped setting of the rest of the parameters in the provided mail object

        // Attach the provided object to the entity manager just like the save() method would.
        em.merge(mail);

        return mail;
    }
}

It's not quite as clean as I was hoping, and I fear I may have made some mistake about something that's kind of hidden and I won't find out until things blow up. But we'll see.

Danny Wedul
  • 201
  • 2
  • 6
  • you can make your method "default Mail safeSave(Mail mail)" synchronized method so that only 1 thread can access that method – Harish Gupta Jul 27 '19 at 08:15
  • have you seen https://stackoverflow.com/a/32315795/878361 ? – destan Jul 27 '19 at 12:55
  • Synchronized will only work per instance of the service, but the issue will still exist between instances. I saw that other stackoverflow answer, but mine is a bit different. That deals with getting a lock on an existing entry. In my case, I want a lock on the table so that nothing can be inserted between looking for an entry, and inserting it if it doesn't exist. If it does exist, I don't want a lock on that row, either. The `@Lock` annotation only locks on a row, and only works with an associated `@Query`. Maybe I'll have to jump through the hoops to make a custom dao. – Danny Wedul Jul 29 '19 at 16:03

1 Answers1

1

I went with a INSERT INTO ... WHERE NOT EXISTS query, and a custom repo. This is listed above under update 2, but I'm putting it here too so that it's easier to find.

public interface MailDaoCustom {
    Mail safeSave(Mail mail);
}

Updated MailDao to implement it:

public interface MailDao extends CrudRepository<Mail, Long>, MailDaoCustom

And then the impl looks like this:

public class MailDaoImpl implements MailDaoCustom {

    @Autowired private MailDao dao;
    @Autowired private EntityManager em;

    public Mail safeSave(Mail mail) {
        // Store a new mail record only if one doesn't already exist.
        Query uniqueInsert = em.createNativeQuery(
                        "INSERT INTO mail " +
                        "       (transaction_id, ...) " +
                        "SELECT :transactionId, ... " +
                        " WHERE NOT EXISTS (SELECT 1 FROM mail " +
                        "                   WHERE transaction_id = :transactionId) ");
        uniqueInsert.setParameter("transactionId", mail.getTransactionId());
        // Snipped setting of the rest of the parameters in the query
        uniqueInsert.executeUpdate();

        // Now go get the record
        Mail entry = dao.findByTransactionId(mail.getTransactionId());
        // Detach the entry so that we can attach the provided mail object later.
        em.detach(entry);

        // Copy all the data from the db entry into the one provided to this method
        mail.setMailId(entry.getMailId());
        mail.setTransactionId(entry.getTransactionId());
        // Snipped setting of the rest of the parameters in the provided mail object

        // Attach the provided object to the entity manager just like the save() method would.
        em.merge(mail);

        return mail;
    }
}
Danny Wedul
  • 201
  • 2
  • 6