> On Jan. 17, 2015, 12:19 a.m., Xuefu Zhang wrote:
> > spark-client/src/main/java/org/apache/hive/spark/client/JobHandleImpl.java,
> > line 179
> > <https://reviews.apache.org/r/29954/diff/1-2/?file=823286#file823286line179>
> >
> > Sorry I didn't get it, but why?
> > Clarity but not perf is my concern. Here we are notifying listeners
> > with a new Spark job ID, which is done in the for loop, which is
> > synchronized. This means no listener may be added or removed from the
> > listeners. On the other hand, sparkJobIds.add(sparkJobId) seems irrelevant
> > to any changes to listeners, unless I missed anything. I don't understand
> > why either of the two cases might happen as you suggested.
Threads: T1 updating the job handle, T2 adding a listener
Case 1:
Statement 1 (S1): sparkJobIds.add(sparkJobId);
Statement 2 (S2): synchronized (listeners) { /* call
onSparkJobStarted(newSparkJobId) on every listener */ }
Timeline:
T1: executes S1
T2: calls addListener(), new listener is notified of the sparkJobId added above
T1: executes S2. New listener is notified again of new spark job ID.
Case 2:
Invert S1 and S2.
T2: calls addListener()
T1: executes S1. Listener is called with the current state of the handle and
new Spark job ID. Listener checks
`handle.getSparkJobIDs().contains(newSparkJobId)`, check fails.
Those seem pretty easy to understand to me. The current code avoids both of
them.
- Marcelo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29954/#review68513
-----------------------------------------------------------
On Jan. 16, 2015, 11:24 p.m., Marcelo Vanzin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29954/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2015, 11:24 p.m.)
>
>
> Review request for hive, Brock Noland, chengxiang li, and Xuefu Zhang.
>
>
> Bugs: HIVE-9179
> https://issues.apache.org/jira/browse/HIVE-9179
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-9179. Add listener API to JobHandle.
>
>
> Diffs
> -----
>
> spark-client/pom.xml 77016df61a0bcbd94058bcbd2825c6c210a70e14
> spark-client/src/main/java/org/apache/hive/spark/client/BaseProtocol.java
> f9c10b196ab47b5b4f4c0126ad455869ab68f0ca
> spark-client/src/main/java/org/apache/hive/spark/client/JobHandle.java
> e760ce35d92bedf4d301b08ec57d1c2dc37a39f0
> spark-client/src/main/java/org/apache/hive/spark/client/JobHandleImpl.java
> 1b8feedb0b23aa7897dc6ac37ea5c0209e71d573
> spark-client/src/main/java/org/apache/hive/spark/client/RemoteDriver.java
> 0d49ed3d9e33ca08d6a7526c1c434a0dd0a06a67
>
> spark-client/src/main/java/org/apache/hive/spark/client/SparkClientImpl.java
> a30d8cbbaae9d25b1cffdc286b546f549e439545
> spark-client/src/test/java/org/apache/hive/spark/client/TestJobHandle.java
> PRE-CREATION
>
> spark-client/src/test/java/org/apache/hive/spark/client/TestSparkClient.java
> 795d62c776cec5e9da2a24b7d40bc749a03186ab
>
> Diff: https://reviews.apache.org/r/29954/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Marcelo Vanzin
>
>
