> On Feb. 3, 2015, 10:27 a.m., Shwetha GS wrote: > > common/src/main/java/org/apache/falcon/entity/lock/MemoryLocks.java, line 32 > > <https://reviews.apache.org/r/30116/diff/2/?file=843611#file843611line32> > > > > Use some small data like Bool(so that memory used is less) as this is > > constant and not used anywhere
makes sense, will do > On Feb. 3, 2015, 10:27 a.m., Shwetha GS wrote: > > common/src/main/java/org/apache/falcon/entity/lock/MemoryLocks.java, line 42 > > <https://reviews.apache.org/r/30116/diff/2/?file=843611#file843611line42> > > > > you can instantiate this at 'private static MemoryLocks instance' to > > avoid getInstance() synchronised Out of curiousity, advantages of changing this to private static over synchronized getInstance() ? > 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 I am just relying on the concurrent hashmap to handle the syncronization of adding to the map. should be ok right ? > On Feb. 3, 2015, 10:27 a.m., Shwetha GS wrote: > > common/src/main/java/org/apache/falcon/entity/lock/MemoryLocks.java, line 81 > > <https://reviews.apache.org/r/30116/diff/2/?file=843611#file843611line81> > > > > Add UTs for Memory Locks Will do > On Feb. 3, 2015, 10:27 a.m., Shwetha GS wrote: > > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java, > > line 282 > > <https://reviews.apache.org/r/30116/diff/2/?file=843612#file843612line282> > > > > tokenList can never be empty? Yes, the token list will not be empty. Its redundant. > 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? It might. In acquire lock we add the entry into a concurrent hash map. It might throw up in case of concurrent writes. > On Feb. 3, 2015, 10:27 a.m., Shwetha GS wrote: > > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java, > > line 344 > > <https://reviews.apache.org/r/30116/diff/2/?file=843612#file843612line344> > > > > we should get just entity lock(not dependents) at schedule() as well Makes sense > 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? I tried, didn't work. will crosscheck once - 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 > >
