Multiple message listeners to single data store. Efficient design

Question

I have a data store that is written to by multiple message listeners. Each of these message listeners can also be in the hundreds of individual threads.

The data store is a PriorityBlockingQueue as it needs to order the inserted objects by a timestamp. To make checking of the queue of items efficient rather than looping over the queue a concurrent hashmap is used as a form of index.

private Map<String, SLAData> SLADataIndex = new ConcurrentHashMap<String, SLAData>();;
private BlockingQueue<SLAData> SLADataQueue;

Question 1 is this a acceptable design or should I just use the single PriorityBlockingQueue.

Each message listener performs an operation, these listeners are scaled up to multiple threads.

Insert Method so it inserts into both.

this.SLADataIndex.put(dataToWrite.getMessageId(), dataToWrite);
this.SLADataQueue.add(dataToWrite);

Update Method

this.SLADataIndex.get(messageId).setNodeId(
            updatedNodeId);

Delete Method

SLATupleData data = this.SLADataIndex.get(messageId);
//remove is O(log n)
this.SLADataQueue.remove(data);
// remove from index
this.SLADataIndex.remove(messageId);

Question Two Using these methods is this the most efficient way? They have wrappers around them via another object for error handling.

Question Three Using a concurrent HashMap and BlockingQueue does this mean these operations are thread safe? I dont need to use a lock object?

Question Four When these methods are called by multiple threads and listeners without any sort of synchronized block, can they be called at the same time by different threads or listeners?

You need to sync the access to Queue and Index. Both stores my be inconsistent otherwise. — Konrad, Jul 31 '15 at 09:38
@Konrad but the operations happen in a single method, and as they are both concurrent collections they will insert anyway. The method call will complete fully for each thread even if others call it, from what I have read anyway? For safety i can do the lock, but would want to know for sure, as unnecessary locking will reduce performance. — perkss, Jul 31 '15 at 10:46
As long as both collections are thread-safe (you didn't mention what kind of map you use exactly), it seems quite okey. You may expect some inconsistency between both collections (map thinks there is an object, queue does not contain it anymore), you have to figure out if it is a problem for you or not. Your solution seems to improve performance only when there are a lot of updates, as deletion and insert take more time than on queue only ... — cichystefan, Jul 31 '15 at 13:22
Please ask one question per post. Some of these questions belong on [Code Review SE](http://codereview.stackexchange.com/), because you have already solved the problem and just want feedback about the "acceptability" (can you be more specific?) of your design. Some of the questions, such as the question about synchronized blocks, do belong here. — Rainbolt, Jul 31 '15 at 13:42

score 0 · Accepted Answer · edited May 23 '17 at 12:15

Question 1 is this a acceptable design or should I just use the single PriorityBlockingQueue.

Certainly you should try to use a single Queue. Keeping the two collections in sync is going to require a lot more synchronization complexity and worry in your code.

Why do you need the Map? If it is just to call setNodeId(...) then I would have the processing thread do that itself when it pulls from the Queue.

// processing thread
while (!Thread.currentThread().isInterrupted()) {
   dataToWrite = queue.take();
   dataToWrite.setNodeId(myNodeId);
   // process data
   ...
}

Question Two Using these methods is this the most efficient way? They have wrappers around them via another object for error handling.

Sure, that seems fine but, again, you will need to do some synchronization locking otherwise you will suffer from race conditions keeping the 2 collections in sync.

Question Three Using a concurrent HashMap and BlockingQueue does this mean these operations are thread safe? I dont need to use a lock object?

Both of those classes (ConcurrentHashMap and the BlockingQueue implementations) are thread-safe, yes. BUT since there are two of them, you can have race conditions where one collection has been updated but the other one has not. Most likely, you will have to use a lock object to ensure that both collections are properly kept in sync.

Question Four When these methods are called by multiple threads and listeners without any sort of synchronized block, can they be called at the same time by different threads or listeners?

That's a tough question to answer without seeing the code in question. For example. someone might be calling Insert(...) and has added it to the Map but not the queue yet, when another thread else calls Delete(...) and the item would get found in the Map and removed but the queue.remove() would not find it in the queue since the Insert(...) has not finished in the other thread.

Multiple message listeners to single data store. Efficient design

1 Answers1