> On Feb. 3, 2015, 10:27 a.m., Shwetha GS wrote: > > common/src/main/java/org/apache/falcon/entity/lock/MemoryLocks.java, line 57 > > <https://reviews.apache.org/r/30116/diff/2/?file=843611#file843611line57> > > > > this should be synchronised so that 2 threads will not get the same lock > > Suhas Vasu wrote: > I am just relying on the concurrent hashmap to handle the syncronization > of adding to the map. should be ok right ? > > Shwetha GS wrote: > Take an example of 2 threads trying to acquire lock for the same entity > at the same time with this sequence of execution(assuming its not already > locked): > t1 -> !locks.containsKey(entityName) will return true > t2 -> !locks.containsKey(entityName) will also return true > t1 -> locks.put(entityName, MAP_VALUE_CONSTANT) will get lock > t2 -> locks.put(entityName, MAP_VALUE_CONSTANT) will also get lock > > Since acquireLock() is not synchronised, both t1 and t2 will get the lock > in this case. > > Instead of making the method synchronised, you can use > ConcurrentHashMap.putIfAbsent() and check the return type if lock is > successful. putIfAbsent is atomic
oh, I missed this scenario > On Feb. 3, 2015, 10:27 a.m., Shwetha GS wrote: > > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java, > > line 334 > > <https://reviews.apache.org/r/30116/diff/2/?file=843612#file843612line334> > > > > acquireLock() doesn't throw exception? > > Suhas Vasu wrote: > It might. In acquire lock we add the entry into a concurrent hash map. It > might throw up in case of concurrent writes. > > Shwetha GS wrote: > acquireLock() signature doesn't define any exception. When is the > exception thrown? Have removed this as the signature of the underlying method has changed. > On Feb. 3, 2015, 10:27 a.m., Shwetha GS wrote: > > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java, > > line 354 > > <https://reviews.apache.org/r/30116/diff/2/?file=843612#file843612line354> > > > > Update API actually takes longer because of dryRun that you should be > > able to add IT. Can you check please? > > Suhas Vasu wrote: > I tried, didn't work. will crosscheck once The issue is trying to simulate 2 independent & simultaneous update commands. The code gets too complicated while trying to achieve this. - Suhas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30116/#review70728 ----------------------------------------------------------- On Feb. 2, 2015, 12:51 p.m., Suhas Vasu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30116/ > ----------------------------------------------------------- > > (Updated Feb. 2, 2015, 12:51 p.m.) > > > Review request for Falcon. > > > Repository: falcon-git > > > Description > ------- > > In distributed mode, when parallel update command are issued for same entity, > they are not syncronized. > This leads to multiple coordinators being spawned in Oozie. > > We need to ensure that the update method is syncronized. > > > Diffs > ----- > > common/src/main/java/org/apache/falcon/entity/lock/MemoryLocks.java > PRE-CREATION > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java > 0d34ef3 > > Diff: https://reviews.apache.org/r/30116/diff/ > > > Testing > ------- > > UT's were added and were successful > > > Thanks, > > Suhas Vasu > >
