Multiple Threads trying to access a Map.

When two threads call the getModels at the same time, then both will have same result.
Then both the threads will try to remove same Map entries at the same time/different time.
To avoid this, I need synchronization of this method. please no synchronized on this method.
I want synchronization only per orderId, so that other orderId's are not unaffected by this synchronization. How can I achieve this using Java synchronization?
@Override
    public List<FModel> getModels(final String orderId) {
        List<FModel> result = new ArrayList<FModel>();
        Session session = txManager.getSession();
        ObjectQuery query = session.createObjectQuery("SELECT f " +
                                                      "FROM Models f " +
                                                      "WHERE f.orderId=?1");
        query.setParameter(1, orderId);
        Iterator iter = query.getResultIterator();
        while (iter.hasNext()) {
            result.add((FModel) iter.next());
        }
        return result;
    }

    @Override
    public void removeModelsForOrder(final String orderId) {
    	
	        List<FModel> modelList = getModels(orderId);
	        ObjectMap modelsMap = getModelsMap();
	        for (FModel model : modelList) {
	            try {
	                ModelsMap.remove(model.getCorrelationId());
	            } catch (ObjectGridException e) {
	                throw new DaoException("Could not remove model for order " + model.getOrderId() +
	                        ", correlation id: " + model.getCorrelationId(), e);
	            }
	        }
    }

Open in new window

srikanthradAsked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

mnrzCommented:
use java.util.concurrent.ConcurrentHashMap instead of your Map

private ConcurrentHashMap map = new ConcurrentHashMap()
0
mnrzCommented:
Also remove 'final' modifier from orderId... however I'm not sure about that whether it makes problem or not but I think you can remove that final modifier.

if you want to synchronize threads per order IDs it's a little complex. You need to use java.util.concurrent.Lock object to lock a specific lines. But I think that ConcurrentHashMap works fine
0
srikanthradAuthor Commented:
That is an ObjectMap from ObjectGrid caching technology. I can't change that.
I am thinking more like the one in attachment.
What do you think of that?
private final static Map<String, String> concurrentHashMap = new ConcurrentHashMap<String, String>();

@Override
    public List<FModel> getModels(final String orderId) {
        List<FModel> result = new ArrayList<FModel>();
        Session session = txManager.getSession();
        ObjectQuery query = session.createObjectQuery("SELECT f " +
                                                      "FROM Models f " +
                                                      "WHERE f.orderId=?1");
        query.setParameter(1, orderId);
        Iterator iter = query.getResultIterator();
        while (iter.hasNext()) {
            result.add((FModel) iter.next());
        }
        return result;
    }

    @Override
    public void removeModelsForOrder(final String orderId) {
        while(concurrentHashMap.containsKey(orderId));
    		concurrentHashMap.put(orderId, orderId);
    		try {
                List<FModel> modelList = getModels(orderId);
                ObjectMap modelsMap = getModelsMap();
                for (FModel model : modelList) {
                    try {
                        ModelsMap.remove(model.getCorrelationId());
                    } catch (ObjectGridException e) {
                        throw new DaoException("Could not remove model for order " + model.getOrderId() +
                                ", correlation id: " + model.getCorrelationId(), e);
                    }
                }
               } finally {
            	   concurrentHashMap.remove(orderId);
    	       }
    }

Open in new window

0
Rowby Goren Makes an Impact on Screen and Online

Learn about longtime user Rowby Goren and his great contributions to the site. We explore his method for posing questions that are likely to yield a solution, and take a look at how his career transformed from a Hollywood writer to a website entrepreneur.

mnrzCommented:
However because the getModel() method only reads data I don't think it has problem but what I doubt is about that Session and the createQuery() method... so if you used ConcurrentHashMap and still you get the same models from getModel() you need to use lock as following codes and you can't lock it per user ID

private final Lock lock = new ReentrantLock();

@Override
    public List<FModel> getModels(final String orderId) {
        List<FModel> result = new ArrayList<FModel>();
        lock.lock();
        try{
        Session session = txManager.getSession();
        ObjectQuery query = session.createObjectQuery("SELECT f " +
                                                      "FROM Models f " +
                                                      "WHERE f.orderId=?1");
        query.setParameter(1, orderId);
        Iterator iter = query.getResultIterator();
        }finally {
          lock.unlock();
        }
        while (iter.hasNext()) {
            result.add((FModel) iter.next());
        }
        return result;
    }

    @Override
    public void removeModelsForOrder(final String orderId) {
        
                List<FModel> modelList = getModels(orderId);
                ObjectMap modelsMap = getModelsMap();
                for (FModel model : modelList) {
                    try {
                        ModelsMap.remove(model.getCorrelationId());
                    } catch (ObjectGridException e) {
                        throw new DaoException("Could not remove model for order " + model.getOrderId() +
                                ", correlation id: " + model.getCorrelationId(), e);
                    }
                }
    }

Open in new window

0
srikanthradAuthor Commented:
Can you please let me know how to use Lock in my specific scenario? Thanks.
0
mnrzCommented:
No the code you attached is not correct and the problem still exists... if you can't change that ObjectMap you can use the another Lock for that section at the time of remove.



private final Lock removelock = new ReentrantLock();

  @Override
    public void removeModelsForOrder(final String orderId) {
        
                List<FModel> modelList = getModels(orderId);
                ObjectMap modelsMap = getModelsMap();
                removelock.lock();
                try {
                for (FModel model : modelList) {
                    try {
                        ModelsMap.remove(model.getCorrelationId());
                    } catch (ObjectGridException e) {
                        throw new DaoException("Could not remove model for order " + model.getOrderId() +
                                ", correlation id: " + model.getCorrelationId(), e);
                    }
                }
                }finally {
                   removelocl.unlock
                }
    }

Open in new window

0
mnrzCommented:
the form of using lock is as follow:

private final Lock lock = new ReentrantLock();
.
.
.
  lock.lock();
  try {
     //code shared among threads
  }finally {
    lock.unlock();
  } 

Open in new window

0
srikanthradAuthor Commented:
I am aware of using Central Locking mechanism that you are doing.
lock.lock();
But, in ObjectGrid the problem is when two threads have the same data before it goes to remove from Map, it is bad, really bad.
1) ModelsMap.remove(model.getCorrelationId());
2) If it doesn't find inside the Map, it queries the database which we don't want at all.

The only way two threads come to this point is when

One thread is coming from Application1 which says mydataSize == App2dataSize. So, go ahead and remove the data from the Map.

Other thread is coming from Application2 which says mydataSize == App1dataSize. So, go ahead and remove the data from the Map.

I can use the synchronized method, but I don't want to synchronize access to the method which affects other orders which does not have the scenario mentioned above.

So, when one thread is coming to the removeModelsForOrder(12345), other threads should wait until the thread that is working on removeModels is done. after which it can resume working again.

When it resumes, it queries the objectMap and it finds no data, so it won't go to the remove() of the Map.
0
srikanthradAuthor Commented:
So, in the scenario you mentioned above.

List<FModel> modelList = getModels(orderId);
                ObjectMap modelsMap = getModelsMap();
                removelock.lock();
                try {
                for (FModel model : modelList) {
                    try {
                        ModelsMap.remove(model.getCorrelationId());
                    } catch (ObjectGridException e) {
                        throw new DaoException("Could not remove model for order " + model.getOrderId() +
                                ", correlation id: " + model.getCorrelationId(), e);
                    }
                }
                }finally {
                   removelocl.unlock
                }

It calls the getModelsMap(orderId) by all the threads. So, all the threads might have similar data/ some data if first thread has already removed some data for that particular order.

So, each and every thread is waiting to remove the particular CorrId from the Map apart from the first thread. When first thread actually completes processing, other threads are going to do stuff which are already done(removed). So, it results in an exception trying to delete something that is already deleted.
0
mnrzCommented:
well that is why I used two lock... check this snippet:

the problem you have stated for ObjectMap
2) If it doesn't find inside the Map, it queries the database which we  don't want at all.

I think before calling remove method you have to check if ID is not exists then remove it...
 map.containsKey(...)

By the way, can you instantiate ObjectMap using ConcurrentHashMap as follow?

ObjectMap map = new ConcurrentHashMap()

private final Lock lock = new ReentrantLock();
private final Lock removelock = new ReentrantLock();

@Override
    public List<FModel> getModels(final String orderId) {
        List<FModel> result = new ArrayList<FModel>();
        lock.lock();
        try{
        Session session = txManager.getSession();
        ObjectQuery query = session.createObjectQuery("SELECT f " +
                                                      "FROM Models f " +
                                                      "WHERE f.orderId=?1");
        query.setParameter(1, orderId);
        Iterator iter = query.getResultIterator();
        }finally {
          lock.unlock();
        }
        while (iter.hasNext()) {
            result.add((FModel) iter.next());
        }
        return result;
    }




  @Override
    public void removeModelsForOrder(final String orderId) {
        
                List<FModel> modelList = getModels(orderId);
                ObjectMap modelsMap = getModelsMap();
                removelock.lock();
                try {
                for (FModel model : modelList) {
                    try {
                        ModelsMap.remove(model.getCorrelationId());
                    } catch (ObjectGridException e) {
                        throw new DaoException("Could not remove model for order " + model.getOrderId() +
                                ", correlation id: " + model.getCorrelationId(), e);
                    }
                }
                }finally {
                   removelocl.unlock
                }
    }

Open in new window

0
mnrzCommented:
I think I figured out your problem.... now check this snippet code:

Hope this works

 @Override
    public void removeModelsForOrder(final String orderId) {
        
                List<FModel> modelList = getModels(orderId);
                
                ObjectMap modelsMap = getModelsMap();
                try {
                for (FModel model : modelList) {
                    removelock.lock();
                    try {
   		      if(modelsMap.containsKey(model.getCorrelationId()) {	
                          ModelsMap.remove(model.getCorrelationId());
                       }
                    }finally {
                      removelocl.unlock
                    }


                }
    }

Open in new window

0
mnrzCommented:
sorry I removed your try-catch block only to clear the code... and there is a inappropriate try { mistakenly I put
0
srikanthradAuthor Commented:
Your containsKey makes sense. But,

if(modelsMap.containsKey(model.getCorrelationId()) hits the database.
I made this mistake long back according to cache it represents database.
So, if the key is not present in the Map, it will go and check the database and fetch the entry from there.
0
srikanthradAuthor Commented:
The Map is an ObjectMap not an ordinary Map. ObjectMap

 private ObjectMap getModelsMap() {
        try {
            return txManager.getSession().getMap(MAP_NAME);
        } catch (UndefinedMapException e) {
            throw new DaoException(MAP_NAME + "is not defined!", e);
        }
    }


and also why my code is not working(sometimes)?

Code is attached.

private final static Map<String, String> concurrentHashMap = new ConcurrentHashMap<String, String>();

    private synchronized void containsKey(String orderId) {
    	while(concurrentHashMap.containsKey(orderId));
    	concurrentHashMap.put(orderId, orderId);
    }

@Override
    public List<FModel> getModels(final String orderId) {
        List<FModel> result = new ArrayList<FModel>();
        Session session = txManager.getSession();
        ObjectQuery query = session.createObjectQuery("SELECT f " +
                                                      "FROM Models f " +
                                                      "WHERE f.orderId=?1");
        query.setParameter(1, orderId);
        Iterator iter = query.getResultIterator();
        while (iter.hasNext()) {
            result.add((FModel) iter.next());
        }
        return result;
    }

    @Override
    public void removeModelsForOrder(final String orderId) {
        containsKey(orderId);
                try {
                List<FModel> modelList = getModels(orderId);
                ObjectMap modelsMap = getModelsMap();
                for (FModel model : modelList) {
                    try {
                        ModelsMap.remove(model.getCorrelationId());
                    } catch (ObjectGridException e) {
                        throw new DaoException("Could not remove model for order " + model.getOrderId() +
                                ", correlation id: " + model.getCorrelationId(), e);
                    }
                }
               } finally {
                   concurrentHashMap.remove(orderId);
               }
    }

Open in new window

0
srikanthradAuthor Commented:
The way I am testing this is using ExecutorService.

@setUp() 
{
        modelDao.persistModel(model5);
        modelDao.persistModel(model6);
        modelDao.persistModel(model7);
        modelDao.persistModel(model8);
}

    class Threads2RemoveOrder implements Callable<Integer>{
    	public Integer call() {
    		modelsDao.removeModelForOrder(orderId3);
    		return modelsDao.getModel(orderId3).size();
    	}
    }

    @Test
    public void threadToRemoveOrderTest() throws InterruptedException, ExecutionException, TimeoutException {
    	
    	ExecutorService service = Executors.newFixedThreadPool(5);
    	
    	Future<Integer> retVal1 = service.submit(new Threads2RemoveOrder());
    	Future<Integer> retVal2 = service.submit(new Threads2RemoveOrder());
    	Future<Integer> retVal3 = service.submit(new Threads2RemoveOrder());
    	Future<Integer> retVal4 = service.submit(new Threads2RemoveOrder());
    	Future<Integer> retVal5 = service.submit(new Threads2RemoveOrder());
    	
    	long timeout = 1000;
    	
        assertEquals(0, retVal1.get(timeout, TimeUnit.MILLISECONDS).intValue());
        assertEquals(0, retVal2.get(timeout, TimeUnit.MILLISECONDS).intValue());
        assertEquals(0, retVal3.get(timeout, TimeUnit.MILLISECONDS).intValue());
        assertEquals(0, retVal4.get(timeout, TimeUnit.MILLISECONDS).intValue());
        assertEquals(0, retVal5.get(timeout, TimeUnit.MILLISECONDS).intValue());
    }

Open in new window

0
mnrzCommented:
if this map always contact the database to load data then you don't have to use that in this case... You need to have the same data in another map which has nothing to do with databases but always you must synchronize those data in your map to those in your ObjectMap  and only if  it is necessary, you can simply remove it from the ObjectMap


0
mnrzCommented:
How about this:
consider you have defined a ConcurretHashMap and each time you keep it up-to-date with ObjectMap

 @Override
    public void removeModelsForOrder(final String orderId) {
        
                List<FModel> modelList = getModels(orderId);
                
                ObjectMap modelsMap = getModelsMap();
 		 
                for (FModel model : modelList) {
                    removelock.lock();
                    try {
   		      if(concurrentMap.containsKey(model.getCorrelationId()) {	
                          ModelsMap.remove(model.getCorrelationId());
			  concurrentMap.remove(model.getCorrelationId());
                       }
                    }finally {
                      removelocl.unlock
                    }


                }
    }

Open in new window

0
mnrzCommented:
private synchronized void containsKey(String orderId)

this method you defined is incorrect... the while loop won't end sometimes!!!!
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
srikanthradAuthor Commented:
Thanks.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Java

From novice to tech pro — start learning today.