[GitHub] drill pull request #921: DRILL-4286 Graceful shutdown of drillbit

[sudheeshkatkam <gi...@git.apache.org>]

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

    https://github.com/apache/drill/pull/921#discussion_r141980790
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/coord/local/LocalClusterCoordinator.java ---
    @@ -85,13 +88,62 @@ public void unregister(final RegistrationHandle handle) {
         endpoints.remove(handle);
       }
     
    +  /**
    +   * Update drillbit endpoint state. Drillbit advertises its
    +   * state. State information is used during planning and initial
    +   * client connection phases.
    +   */
    +  @Override
    +  public RegistrationHandle update(RegistrationHandle handle, State state) {
    +    DrillbitEndpoint endpoint = handle.getEndPoint();
    +    endpoint = endpoint.toBuilder().setState(state).build();
    +    handle.setEndPoint(endpoint);
    +    endpoints.put(handle,endpoint);
    +    return handle;
    +  }
    +
       @Override
       public Collection<DrillbitEndpoint> getAvailableEndpoints() {
         return endpoints.values();
       }
     
    +  /**
    +   * Get a collection of ONLINE Drillbit endpoints by excluding the drillbits
    +   * that are in QUIESCENT state (drillbits shutting down). Primarily used by the planner
    +   * to plan queries only on ONLINE drillbits and used by the client during initial connection
    +   * phase to connect to a drillbit (foreman)
    +   * @return A collection of ONLINE endpoints
    +   */
    +  @Override
    +  public Collection<DrillbitEndpoint> getOnlineEndPoints() {
    +    Collection<DrillbitEndpoint> runningEndPoints = new ArrayList<>();
    +    for (DrillbitEndpoint endpoint: endpoints.values()){
    +      if(endpoint.getState().equals(State.ONLINE)) {
    --- End diff --
    
    This check should be:
    `if(!endpoint.hasState() || endpoint.getState().equals(State.ONLINE)) {`
    to not break backward compatibility (new client and old cluster of bits). So the assumption is that old server is ONLINE.


---