Search code examples
jakarta-eeejbejb-3.1

EJB: Lock access to a method only instead of whole bean instance


In my application I am providing some application wide data in a @Singleton EJB. The preperation of the data is a long running task. However, the underlying source of this outcome is changing frequently, so I have to recalculated the data which is populated by the Singleton frequently as well.

The task now is to ensure a quick access to the data, even if it is currently recalculated. It does not matter if I am exposing the old status, while the preparation is ongoing.

My current approach looks like this:

@Singleton
@Startup
@Lock(LockType.READ)
public class MySingleton {

    @Inject private SomeService service;       
    @Getter private SomeData currentVersion;

    @PostConstruct
    private void init() {
        update();
    }

    private void update() {
        currentVersion = service.longRunningTask();
    }

    @Lock(LockType.WRITE)
    @AccessTimeout(value = 1, unit = TimeUnit.MINUTES)
    @Asynchronous
    public void catchEvent(
                 @Observes(during = TransactionPhase.AFTER_SUCCESS) MyEvent event) {
        this.update();            
    }        
}

The idea behind this is to prepare the data once on startup. Afterwards the clients are able to access the current version of my data (consider this data as immutable) concurrently.

Now, if some action happens that may cause a recalculation of the data, I am fireing a CDI event and in the Singleton I am observing this event and triggering the recalculation again. Those actions may happen quite often in a short period of time (but also there might be none of those actions for a long time).

  • I marked the method as @Asynchronous so it is executed, so other clients may still access the getter concurrently in different thread of execution.
  • I marked it as LockType.Write and with the AccessTimeout, because I want to drop redundant events, when they are occuring in a short period of time.

The Problem is obvious:

  • If I am using the LockType.WRITE then the access to the getter methods is locked as well, during the recalculation.
  • If I am not using it I while end up in heavy ressource consumption when there are many CDI events observed

Is it possible to lock the access to this single method only instead of locking access to the whole instance of my singleton? Would this solve my problem? Any other approaches how I may handle my problem?

How to deal with this?


Solution

  • Option A: Never have more than 1 thread waiting:

    • Try to get a WriteLock for this method only. If it getting the lock succeeds, update and unlock.
    • If tryLock does not succeed, check if there is already a thread waiting to get the WriteLock.
      • If there is a thread already waiting, abort the method since we don't want 2 threads waiting on the WriteLock at the same time.
      • If there are not other threads waiting to get a lock, wait on a WriteLock and then update.

    .

    @Singleton
    @Startup
    @Lock(LockType.READ)
    public class MySingleton {
    
        private ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
    
        // ...
    
        @Asynchronous
        public void catchEvent(@Observes(during = TransactionPhase.AFTER_SUCCESS) MyEvent event) 
        {
            try{
                if(!lock.writeLock().tryLock()){
                    // Unable to immediately obtain lock
    
                    if(lock.hasQueuedThreads()){
                        // there is *probably* at least 1 thread waiting on
                        // the lock already, don't have more than 1 thread waiting
                        return;
                    } else {
                        // There are no other threads waiting.  Wait to acquire lock
                        lock.writeLock().lock();
                        this.update();
                    }
                } else {
                    // obtained the lock on first try
                    this.update();
                }
            } finally {
                try {
                    lock.writeLock().unlock();
                } catch (IllegalMonitorStateException ignore){}
            }
        }        
    }
    

    Note: Brett Kail pointed out that there could be a race condition here where two threads go through the tryLock() and lock.hasQueuedThreads() at the same time and one of them gets the write lock while the other blocks for the full duration of this.update(). This could potentially happen, but I think this is rare enough that the advantage of not waiting at all most of the time outweighs waiting 1 minute most of the time.


    Option B: Never have any threads waiting:
    I'm not sure how steadily your events come in, but it would simplify the logic much more if you just aborted if tryLock() returned false, however you wouldn't get as many updates.

    @Asynchronous
    public void catchEvent(@Observes(during = TransactionPhase.AFTER_SUCCESS) MyEvent event) 
    {
        if(!lock.writeLock().tryLock())
            return; // there is already an update processing
    
        try{
            this.update();
        } finally {
            lock.writeLock().unlock();
        }
    }
    

    Option C: Wait for up to X amount of time for a lock (mimmic @AccessTimeout)
    Brett pointed out in the comments that option A has a race condition and could potentially have a thread block for the full duration of this.update(), and suggested this approach as a safer alternative. The only drawback is that tryLock() will end up timing out most of the time, so setting the timeout as small as possible is advantageous.

    @Asynchronous
    public void catchEvent(@Observes(during = TransactionPhase.AFTER_SUCCESS) MyEvent event) 
    {
    
        if(lock.writeLock().tryLock(1, TimeUnit.MINUTES)) {
            try{
                this.update();
            } finally {
                lock.writeLock().unlock();
            }
        }
    }