> On Oct. 27, 2017, 12:25 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPool.java
> > Lines 120 (patched)
> > <https://reviews.apache.org/r/63230/diff/2/?file=1867959#file1867959line131>
> >
> >     Alternatively, since you are not looking at return value anyway. You 
> > can use CompletableFuture.allOf() which will returns one combined 
> > CompletableFuture which can be used to block.

FutureTask is not a completablefuture


> On Oct. 27, 2017, 12:25 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmTezSession.java
> > Lines 83 (patched)
> > <https://reviews.apache.org/r/63230/diff/2/?file=1867964#file1867964line87>
> >
> >     This could be doable without Guava. With JDK8 
> > CompletableFuture.acceptEither.

ScheduledFuture is not a completablefuture either... also it requires 
completionstage on top of that, which just by the length of the javadoc 
introduction seems to be designed for much more complex stuff :)


> On Oct. 27, 2017, 12:25 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
> > Line 234 (original), 292 (patched)
> > <https://reviews.apache.org/r/63230/diff/2/?file=1867965#file1867965line326>
> >
> >     same here. awaitUninterruptibly()

Same reason :)


> On Oct. 27, 2017, 12:25 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
> > Lines 491 (patched)
> > <https://reviews.apache.org/r/63230/diff/2/?file=1867965#file1867965line525>
> >
> >     When will init be cancelled?
> >     
> >     If admin makes changes to pool, the event gets propagated and start 
> > with init state. Now if admin makes another change immediately (before the 
> > previous one completes), would that cancel the previous init?
> >     
> >     Is applying EventState synchronous?

The only way to cancel init is from this thread; see pool.update and 
pool.destroy. So, if the pool is removed, pool.destroy would always be called 
in the same iteration with setting init state to cancelled.


> On Oct. 27, 2017, 12:25 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
> > Lines 503 (patched)
> > <https://reviews.apache.org/r/63230/diff/2/?file=1867965#file1867965line537>
> >
> >     Assuming this happens when there is no capacity in cluster for spinning 
> > up new sessios (or could be other reasons)? Is there a way to determine why 
> > this failed and throw error if not restartable (may be some permission 
> > issue)?

It actually says internal error ;) This means the pool has been removed without 
init being marked as cancelled. Made the statement more specific


> On Oct. 27, 2017, 12:25 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
> > Lines 1138 (patched)
> > <https://reviews.apache.org/r/63230/diff/2/?file=1867965#file1867965line1339>
> >
> >     Create follow jira to handle this case? Killing all seems like overkill 
> > :P
> >     
> >     We could kill most recently submitted queries that will satisfy delta. 
> > Or make it optional to the user to pick the policy they want (pluggable).

Will file a JIRA; this was a decision when we were discussing w/Carter and 
Gunther... because killing only some is "too complicated" :)


> On Oct. 27, 2017, 12:25 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
> > Lines 1262 (patched)
> > <https://reviews.apache.org/r/63230/diff/2/?file=1867965#file1867965line1463>
> >
> >     is this a recoverable state. If not should session be set to null here?

no, not recoverable... also if state is invalid master thread cannot handle it. 
It probably means someone added an enum constant.


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63230/#review189373
-----------------------------------------------------------


On Oct. 24, 2017, 11:58 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63230/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2017, 11:58 p.m.)
> 
> 
> Review request for hive and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 0b4abb824f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/AmPluginNode.java 35d380c92b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/GuaranteedTasksAllocator.java 
> d978a25b14 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/LlapPluginEndpointClientImpl.java
>  45c3e38dcc 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/QueryAllocationManager.java 
> a326db3ab0 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SessionExpirationTracker.java 
> da93a3a791 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPool.java 
> b67c933b19 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 
> 9b4714f1d7 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolSession.java 
> 613522357e 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 
> 144816862d 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/UserPoolMapping.java 
> 81d6b859a6 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmTezSession.java 00501eef93 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 
> d725e90475 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/LlapClusterStateForCompile.java
>  209cf57a6a 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/SampleTezSessionState.java 
> 59efd43be6 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java 
> 258a865fef 
> 
> 
> Diff: https://reviews.apache.org/r/63230/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Reply via email to