7

I have multiple quartz workers Each worker picks a db record (printer) and then doing some work on it (reading info from the printer over the network).
It can take up to 30 sec to 1 min to complete each job.

Back in JDBC days I would run (pseudo code)

     printer = "select from printer where status=? for update"
     do the work, (takes 1 min)
     update the printer record.

My question is this approach with PESSIMISTIC_WRITE is ok:

public interface PrinterRepo extends CrudRepository<Printer,String>
 {
 @Lock(LockModeType.PESSIMISTIC_WRITE)
 @Query("SELECT r FROM Printers r where r.status = :status")
 Printer findOneAndLock(@Param("status")String status);
}

Then the worker:

@Transactional
public void execute(JobExecutionContext jobExecutionContext) {
      Printer p = printerRepo.findOneAndLock("status");
     //do the work here (30 sec to 1 min)
     printerRepo.save(p);
 }

For my understanding lock will be release at the end of the function annotated with @Transactional correct? My question is what will happen to other workers?
Will they starve while waiting for findOneAndLock?

Thank you

JavaSheriff
  • 7,074
  • 20
  • 89
  • 159

1 Answers1

5

Regardless of which type and level of locks you are going to use, and what will happen to other workers, the long-term lock, as well as long-term transaction, is not a good solution. IMHO in your case is better to use different approach without any locks, for example, an additional table to record the printer 'locks':

create table printer_locks (
  printer_id bigint    not null constraint printer_locks_pkey primary key,
  created_at timestamp not null,
  worker_id  bigint    not null constraint fk_printer_locks_worker_id references workers,

  constraint fk_printer_locks_printer_id foreign key (printer_id) references printers(id)
);

When a worker wants to start the job with some printer, first it tries to insert the record to this table. Then, in case of success, it starts the job. When the job is completed then the worker deletes this record.

Because the printer_id column is unique - other workers will not be able to start working with the same printer at the same time.

Implementation:

@Entity
@Table(name = "workers")
public class Worker {

   @Id 
   @GeneratedValue(...)
   private Long id;
   // other stuff...
}
@Entity
@Table(name = "printers")
public class Printer {

   @Id 
   @GeneratedValue(...)
   private Long id;
   // other stuff...
}
@Entity
@Table(name = "printer_locks")
public class PrinterLock {

   @Id 
   private Long id;

   private Instant createdAt = Instant.now();

   @OneToOne(fetch = FetchType.LAZY)
   @MapsId
   private Printer printer;

   @ManyToOne(fetch = FetchType.LAZY)
   private Worker worker;

   public PrinterLock(Printer printer, Worker worker) {
       this.printer = printer;
       this.worker = worker;
   } 

   // other stuff...
}
public void execute(...) {

     Worker worker = ...;
     Long printerId = ...;

     printerRepo.findById(printerId)
         .map(printer -> {
             try {
                 printerLockRepo.save(new PrinterLock(printer, worker)); 
                 try {
                     // do the work here (30 sec to 1 min)
                 } finally {
                     printerLockRepo.deleteById(printerId); 
                 }
             } catch(Exception e) {
                log.warn("Printer {} is busy", printerId);
             }         
         })
         .orElseThrow(() -> new PrinterNotFoundException(printerId));
 }

Note that the execute method even doesn't have @Transactional annotation.

An additional advantage of this approach is the column createdAt which allow you to control hanging jobs.

Further reading:

Cepr0
  • 28,144
  • 8
  • 75
  • 101
  • thank you, the idea is not bad but adds overhead like additional table, foreign keys, and JPA objects, whats wrong with "select for update" modifying the status field on the printer, starting the work and updating at the end of the work? – JavaSheriff May 14 '19 at 18:42
  • I prefer two atomic operations with tiny 'overhead' than three operations, locking, and transaction with a guaranteed big headache ) – Cepr0 May 15 '19 at 16:31
  • Also lets say a worker is scheduled to work every 5 min, he is trying to find by id, then other worker will pick up so save will fail.., first worker will do nothing for 5 min? select for update adds a DB lock so workers will not stay idle... – JavaSheriff May 15 '19 at 21:12
  • @user648026 Why "do nothing for 5 min"? Let it pick up the next printer. – Cepr0 May 16 '19 at 09:32
  • because its a scheduled quartz worker, if it was unable to perform this action: printerLockRepo.save(new PrinterLock(printer, worker)); then it will hang for 5 min. – JavaSheriff May 17 '19 at 13:39