falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shwetha GS" <sshivalingamur...@hortonworks.com>
Subject Re: Review Request 30116: Parallel update APIs create 2 coords
Date Mon, 09 Feb 2015 11:52:50 GMT


> 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
> 
> Suhas  Vasu wrote:
>     Out of curiousity, advantages of changing this to private static over synchronized
getInstance() ?

getInstance() is called from a lot of places. Instead of synchronising each call, initialising
at static variable(which is guaranteed to be called once) is better.


> 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 ?

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


> 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.

acquireLock() signature doesn't define any exception. When is the exception thrown?


- Shwetha


-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message