ExecutorService fairness

Question

I have an ExecutorService like this

ExecutorService executor =  new ThreadPoolExecutor(threads, threads,
            0L, TimeUnit.MILLISECONDS,
            new ArrayBlockingQueue<>(1000, true));

and i'm sending work to it with .execute(Runnable)

My runnable has

        @Override
        public void run() {
            this.processor.performAction(this);
        }

where processor has

public void performAction(RunnableAction action) {
        Lock lock = lockManager.getLock(action.getId());
        lock.lock();
        try {
            action.execute();
        } finally {
            lock.unlock();
        }
    }

where the lockManager is

public class LockManager {

    Map<String, Lock> lockById = new HashMap<>(1000);

    public synchronized Lock getLock(String id) {
        Lock lock = lockById.get(id);
        if (lock == null) {
            lock = new ReentrantLock(true);
        }
        return lock;
    }

and my actions/runnable have an execQty field that triggers an update on some orders calling their order.execute(execQty)

public void execute(int execQty) {
        if (execQty < this.lastExecQty) {
            System.err.println("<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>");
            System.err.println("id: " + this.id + " wrong order" + this.lastExecQty + " > " + execQty);
        }
        this.execQty += execQty;
        this.lastExecQty = execQty;
    }
}

I have ordered my runnables to send to the scheduler always having the qty field each bigger than the previous, so when printing each runnable before sending it to the ExecutorService i always get what i need, ordered numbers:

execute: id: 49  qty: 819
execute: id: 49  qty: 820
execute: id: 49  qty: 821
execute: id: 49  qty: 822
execute: id: 49  qty: 823

But even though my ExecutorService is backed by a fair queue, and i'm using per entity locks before each entity update it still seems the entity is not updated in order

execute: id: 88  qty: 6
execute: id: 88  qty: 7
execute: id: 88  qty: 8
execute: id: 88  qty: 9
execute: id: 88  qty: 10
execute: id: 88  qty: 11
execute: id: 88  qty: 12
execute: id: 88  qty: 13
execute: id: 88  qty: 14
execute: id: 88  qty: 15
execute: id: 88  qty: 16
execute: id: 88  qty: 17
execute: id: 88  qty: 18
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
id: 88 wrong order 17 (previous) > 7 (current)
execute: id: 88  qty: 19
execute: id: 88  qty: 20

It works fine when running the ExecutorService with just one thread

Answer 1

Your LockManager doesn't seem right. You are never putting those locks into the map, so you are always synchronizing on a new object.

Suggested change:

public synchronized Lock getLock(String id) {
    Lock lock = lockById.get(id);
    if (lock == null) {
        lock = new ReentrantLock(true);
        // put lock into the map so that next one will reuse it
        lockById.put(id, lock);
    }
    return lock;
}