Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/921#discussion_r150971745
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
 ---
    @@ -200,11 +206,50 @@ public void unregister(RegistrationHandle handle) {
         }
       }
     
    +  /**
    +   * Update drillbit endpoint state. Drillbit advertises its
    +   * state in Zookeeper when a shutdown request of drillbit is
    +   * triggered. State information is used during planning and
    +   * initial client connection phases.
    +   */
    +  public RegistrationHandle update(RegistrationHandle handle, State state) 
{
    +    ZKRegistrationHandle h = (ZKRegistrationHandle) handle;
    +      try {
    +        endpoint = h.endpoint.toBuilder().setState(state).build();
    +        ServiceInstance<DrillbitEndpoint> serviceInstance = 
ServiceInstance.<DrillbitEndpoint>builder()
    +                .name(serviceName)
    +                .id(h.id)
    +                .payload(endpoint).build();
    +        discovery.updateService(serviceInstance);
    +      } catch (Exception e) {
    +        e.printStackTrace();
    +      }
    +      return handle;
    +  }
    +
       @Override
       public Collection<DrillbitEndpoint> getAvailableEndpoints() {
         return this.endpoints;
       }
     
    +  /*
    +   * 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){
    +      if(!endpoint.hasState() || endpoint.getState().equals(State.ONLINE)) 
{
    --- End diff --
    
    This stanza appears multiple times. Can we define a static function that 
does the double check to avoid redundant code?
    
    ```
    boolean isInState(State state) {
    ...
    ```
    Choose an appropriate name, maybe `isDrillbitInState` or whatever.


---

Reply via email to