drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Rogers <par0...@yahoo.com.INVALID>
Subject Re: [DISCUSS] ExecConstants class refactoring
Date Mon, 02 Mar 2020 18:01:11 GMT
Hi Igor,

Great idea; I've been noticing that file has gotten excessively large.

I wonder if we can split the file by topic instead of by the (often odd) naming hierarchy
which has evolved over the years. For example, one file for internal server config options
(thread counts, RPC stuff.) Another for things related to local config (local file systems,
etc.) Another related to core operators (sorts, hash joins, etc.) Picking the right split
will require a bit of thought and sone experimentation.


There are three kinds of constants:

* Config variables (from drill-override.conf)
* System/session options
* Other random constants

One could argue that we should keep the three kinds together for each topic. (That is, all
the sort-related stuff in one place.) Whether that means three well-known files in one place,
three nested interfaces within a single class could be debated.

One thing we probably should do is to separate out the string name of a system/session property
from the implementation of its validator. It used to be that people would use the validator
to access the option value. Most newer code in the last several years uses the typed access
methods with a string key. So, we can move the validators into a OptionDefinitions class/interface
separate from the key definitions.

Most names are for the benefit of us: the poor humans who have to understand them. The compiler
would be happy with inline constant values. Most names tend to be short to be easier to remember.
For example, it is easier to understand CLIENT_RPC_THREADS than "drill.exec.rpc.user.client.threads".


Most costants are in ExecConstants. Some (but not all) constants for the planner live in PlannerSettings.
Oddly, some planner settings are in ExecConstants. We might want to consolidate planner-related
constants into a single location.

One final thing to keep in mind is that the "java-exec" project has become overly large. At
some point, it might make sense to split it into components, such as planner, server, exec
engine, etc. So, if our constants are grouped by functional units (rather than by name), it
might make it easier to reshuffle modules if we ever choose to do so.

Thanks,
- Paul

 

    On Monday, March 2, 2020, 8:20:32 AM PST, Igor Guzenko <ihor.huzenko.igs@gmail.com>
wrote:  
 
 Hello Drillers,

I would like to refactor the ExecConstants class in order to group our
property keys by related sections. Neglecting some naming conventions for
the case, we can utilize Java interfaces with fields (which are constants
by default), and create something like:

public interface ExecOption {
  interface http {
    interface session {
      interface memory {
        String maximum = "drill.exec.http.session.memory.maximum";
        String reservation = "drill.exec.http.session.memory.reservation";
      }
    }

    interface jetty {
      interface server {
        String acceptors = "drill.exec.http.jetty.server.acceptors";
        String selectors = "drill.exec.http.jetty.server.selectors";
      }
    }
  }
}

So then with the help of Java imports, we can write client code like:

config.getLong(http.session.memory.reservation),
config.getLong(http.session.memory.maximum)

As a bonus, IDE's autocomplete and class structure features will allow an
exploration of available choices fairly easily, since all related options
will be related to specific nested interfaces instead of one top-level
class.

Any thoughts and suggestions are highly appreciated :)

Thanks,
Igor
  
Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message