Access to shared resource, lock unlock or wait notify

Scenario:

Multi-threads reading from different sources.
One single access point to a shared queue (See a class RiderSynchronized trying to write)
Every line a Reader reads, It tries to insert into a shared queue through method RiderSynchronized provides.

When shared queue is full, I have to run a batch on a prepared statement to insert into Oracle. Meanwhile, all access to shared queue it must be denied.

Code:

public class RiderSynchronized {

    private ArrayDeque<JSONRecord> queue = new ArrayDeque<>();
    private OracleDAO oracleDao;
    private long capacity;

    public RiderSynchronized(OracleDAO oracleDao, long capacity) {
        this.oracleDao = oracleDao;
        this.capacity = capacity;
    }

    public synchronized boolean addRecord(JSONRecord record) {
        boolean success = false;
        try {
            while (queue.size() >= capacity) {
                wait();
            }

            queue.add(record);
            if (queue.size() < capacity) {
                success = true;
                notify(); //notify single Thread
            } else {
                JSONRecord currentRecord = null;
                while ((currentRecord = queue.poll()) != null) {
                    oracleDao.insertRowParsedIntoBatch(currentRecord);
                }
                oracleDao.runBatch();
                success = true;
                notifyAll(); //it could be all Reading Threads are waiting. Notify all

            }

        } catch (Exception e) {
            success = false;
        }
        return success;
    }
}

I have to admit I'm a little worried about a thing.

1) Reader threads can just use addRecord indistinctly? Are They going to wait for themselves? Or Do I have to implement some other method where to check before to run addRecord Method?

2) When queue.size < capacity, I decide to notify just to one thread, because IMHO, at this point, no threads should be in status waiting. Am I wrong? Should I notify All?

2b) Exact question for the "else" statement. Is it a good practice to notifyAll? At this point, it could be all threds are waiting?

3) Finally. I'm a little concerned to re-write everything using Lock e Condition Classes. Is it a better decision? Or Is it ok how I'm running this scenario?

Solution

1) Reader threads can just use addRecord indistinctly? Are They going to wait for themselves? Or Do I have to implement some other method where to check before to run addRecord Method?

The problem with your current code is that if for some reason notifyAll is not called by the only thread that theoretically should be able to go into the else block then your threads will wait forever.

The potential risks in your code are:

oracleDao.insertRowParsedIntoBatch(currentRecord)
oracleDao.runBatch()

With your current code if one of those methods throw an exception notifyAll will never be called so your threads will wait forever, you should at least consider calling notifyAll in a finally block to make sure that it will be called whether happens.

2) When queue.size < capacity, I decide to notify just to one thread, because IMHO, at this point, no threads should be in status waiting. Am I wrong? Should I notify All?

Your threads could only wait in case queue.size() >= capacity so for me notify is not even needed as this condition (queue.size() < capacity) is not expected by any thread.

2b) Exact question for the "else" statement. Is it a good practice to notifyAll? At this point, it could be all threds are waiting?

Item 69 from Effective Java:

A related issue is whether you should use notify or notifyAll to wake waiting threads. (Recall that notify wakes a single waiting thread, assuming such a thread exists, and notifyAll wakes all waiting threads.) It is often said that you should always use notifyAll. This is reasonable, conservative advice. It will always yield correct results because it guarantees that you’ll wake the threads that need to be awakened. You may wake some other threads, too, but this won’t affect the correctness of your program. These threads will check the condition for which they’re waiting and, finding it false, will continue waiting. As an optimization, you may choose to invoke notify instead of notifyAll if all threads that could be in the wait-set are waiting for the same condition and only one thread at a time can benefit from the condition becoming true. Even if these conditions appear true, there may be cause to use notifyAll in place of notify. Just as placing the wait invocation in a loop protects against accidental or malicious notifications on a publicly accessible object, using notifyAll in place of notify protects against accidental or malicious waits by an unrelated thread. Such waits could otherwise “swallow” a critical notification, leaving its intended recipient waiting indefinitely.

3) Finally. I'm a little concerned to re-write everything using Lock e Condition Classes. Is it a better decision? Or Is it ok how I'm running this scenario?

Lock and Condition are interesting if you need features that are not available with intrinsic locks like for example tryLock() or the ability to awake only threads waiting for a given condition. In your case it doesn't seem to be necessary so you can keep it like it is.