You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Purshotam Shah <pu...@yahoo-inc.com> on 2014/07/11 02:55:51 UTC
Review Request 23403: OOZIE-1923 ZKLocksService locks are not re-entrant
like MemoryLocks
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23403/
-----------------------------------------------------------
Review request for oozie.
Bugs: OOZIE-1923
https://issues.apache.org/jira/browse/OOZIE-1923
Repository: oozie-git
Description
-------
OOZIE-1923 ZKLocksService locks are not re-entrant like MemoryLocks
Diffs
-----
core/src/main/java/org/apache/oozie/service/ZKLocksService.java d03a899
core/src/test/java/org/apache/oozie/service/TestZKLocksService.java a773469
Diff: https://reviews.apache.org/r/23403/diff/
Testing
-------
UTC
Thanks,
Purshotam Shah
Re: Review Request 23403: OOZIE-1923 ZKLocksService locks are not re-entrant
like MemoryLocks
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23403/#review48046
-----------------------------------------------------------
Ship it!
Ship It!
- Rohini Palaniswamy
On July 11, 2014, 12:56 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23403/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 12:56 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1923
> https://issues.apache.org/jira/browse/OOZIE-1923
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-1923 ZKLocksService locks are not re-entrant like MemoryLocks
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/service/ZKLocksService.java d03a899
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java a773469
>
> Diff: https://reviews.apache.org/r/23403/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 23403: OOZIE-1923 ZKLocksService locks are not re-entrant
like MemoryLocks
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> On July 16, 2014, 2:57 a.m., Rohini Palaniswamy wrote:
> > core/src/main/java/org/apache/oozie/service/ZKLocksService.java, line 181
> > <https://reviews.apache.org/r/23403/diff/2/?file=628236#file628236line181>
> >
> > Can move val == 0 condition inside synchronized block and remove the TODO block.
That doesn't fix the issue.
Count can be increase after we call lock.getParticipantNodes().size();
Count is increased after we acquire the lock.
- Purshotam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23403/#review47851
-----------------------------------------------------------
On July 11, 2014, 12:56 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23403/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 12:56 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1923
> https://issues.apache.org/jira/browse/OOZIE-1923
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-1923 ZKLocksService locks are not re-entrant like MemoryLocks
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/service/ZKLocksService.java d03a899
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java a773469
>
> Diff: https://reviews.apache.org/r/23403/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 23403: OOZIE-1923 ZKLocksService locks are not re-entrant
like MemoryLocks
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23403/#review47851
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/service/ZKLocksService.java
<https://reviews.apache.org/r/23403/#comment84077>
Can move val == 0 condition inside synchronized block and remove the TODO block.
- Rohini Palaniswamy
On July 11, 2014, 12:56 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23403/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 12:56 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1923
> https://issues.apache.org/jira/browse/OOZIE-1923
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-1923 ZKLocksService locks are not re-entrant like MemoryLocks
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/service/ZKLocksService.java d03a899
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java a773469
>
> Diff: https://reviews.apache.org/r/23403/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 23403: OOZIE-1923 ZKLocksService locks are not re-entrant
like MemoryLocks
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> On July 11, 2014, 7:30 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/service/ZKLocksService.java, line 104
> > <https://reviews.apache.org/r/23403/diff/2/?file=628236#file628236line104>
> >
> > A few questions about this:
> >
> > 1) If a thread acquires a readlock and another thread acquires a writelock, then you'll lose the readlock because they both get stored in the same HashMap indexed by the resource. Perhaps we need two maps? one for read locks and one for write locks?
> >
> > 2) Isn't this going to be very slow? acquiring and releasing any lock requires synchronizing on the zkLocks map. That's a huge bottleneck. I don't have a better idea though; perhaps there's some "fancier" Map from Guava that can help here to make it more granular?
1.
No that will not happen. If you have a readlock, you can't acquire writelock.
For other case, we remove lock only if no one is waiting or holding the lock.
int val = lock.getParticipantNodes().size();
if (val == 0) {
synchronized (zkLocks) {
zkLocks.remove(resource);
}
}
2. No. We are not synchronizing whole acquire block, only when we add/remove entry from hashmap.
- Purshotam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23403/#review47671
-----------------------------------------------------------
On July 11, 2014, 12:56 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23403/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 12:56 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1923
> https://issues.apache.org/jira/browse/OOZIE-1923
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-1923 ZKLocksService locks are not re-entrant like MemoryLocks
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/service/ZKLocksService.java d03a899
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java a773469
>
> Diff: https://reviews.apache.org/r/23403/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 23403: OOZIE-1923 ZKLocksService locks are not re-entrant
like MemoryLocks
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23403/#review47671
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/service/ZKLocksService.java
<https://reviews.apache.org/r/23403/#comment83800>
A few questions about this:
1) If a thread acquires a readlock and another thread acquires a writelock, then you'll lose the readlock because they both get stored in the same HashMap indexed by the resource. Perhaps we need two maps? one for read locks and one for write locks?
2) Isn't this going to be very slow? acquiring and releasing any lock requires synchronizing on the zkLocks map. That's a huge bottleneck. I don't have a better idea though; perhaps there's some "fancier" Map from Guava that can help here to make it more granular?
- Robert Kanter
On July 11, 2014, 12:56 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23403/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 12:56 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1923
> https://issues.apache.org/jira/browse/OOZIE-1923
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-1923 ZKLocksService locks are not re-entrant like MemoryLocks
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/service/ZKLocksService.java d03a899
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java a773469
>
> Diff: https://reviews.apache.org/r/23403/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 23403: OOZIE-1923 ZKLocksService locks are not re-entrant
like MemoryLocks
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23403/
-----------------------------------------------------------
(Updated July 11, 2014, 12:56 a.m.)
Review request for oozie.
Bugs: OOZIE-1923
https://issues.apache.org/jira/browse/OOZIE-1923
Repository: oozie-git
Description
-------
OOZIE-1923 ZKLocksService locks are not re-entrant like MemoryLocks
Diffs
-----
core/src/main/java/org/apache/oozie/service/ZKLocksService.java d03a899
core/src/test/java/org/apache/oozie/service/TestZKLocksService.java a773469
Diff: https://reviews.apache.org/r/23403/diff/
Testing
-------
UTC
Thanks,
Purshotam Shah