Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/432#discussion_r28477732
  
    --- Diff: tajo-core/src/main/java/org/apache/tajo/master/QueryInfo.java ---
    @@ -119,14 +119,37 @@ public int getQueryMasterClientPort() {
         return queryMasterClientPort;
       }
     
    -  public TajoProtos.QueryState getQueryState() {
    +  public synchronized TajoProtos.QueryState getQueryState() {
         return queryState;
       }
     
    -  public void setQueryState(TajoProtos.QueryState queryState) {
    +  public synchronized boolean isTerminalState() {
    +    return isTerminalState(queryState);
    +  }
    +
    +  public static boolean isTerminalState(TajoProtos.QueryState state) {
    +    return state == TajoProtos.QueryState.QUERY_FAILED ||
    +        state == TajoProtos.QueryState.QUERY_ERROR ||
    +        state == TajoProtos.QueryState.QUERY_KILLED ||
    +        state == TajoProtos.QueryState.QUERY_SUCCEEDED;
    +  }
    +
    +  public synchronized void setQueryState(TajoProtos.QueryState queryState) 
{
         this.queryState = queryState;
    +    notifyAll();
       }
     
    +  public synchronized boolean waitState(TajoProtos.QueryState expect, long 
timeout)
    +      throws InterruptedException {
    +    long prev = System.currentTimeMillis();
    +    while (timeout > 0 && expect != queryState && !isTerminalState()) {
    +      wait(timeout);
    --- End diff --
    
    I think that this approach has a potential problem of process hang. Once 
wait() is called, it can wake up only when setQueryState() is called. During 
query processing, setQueryState() is called only when TajoMaster receives 
heartbeats from workers. That means, if some heartbeats are lost, this process 
cannot wake up even though waiting time exceeds the given timeout. 
    
    This will be not a big problem because we usually expect to receive at 
least one heartbeat from a lot of workers. However, I think that it is still a 
problem because the process's work can be different from our intention.
    
    What do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to