[GitHub] [drill] paul-rogers commented on pull request #2139: DRILL-6268: Drill-on-YARN client obtains HDFS URL incorrectly

[GitBox <gi...@apache.org>]

paul-rogers commented on pull request #2139:
URL: https://github.com/apache/drill/pull/2139#issuecomment-812928381


   @cgivre, Revisiting this one. The fix is probably not correct. As explained earlier, the goal is to 1) use the YARN config by default, unless, 2) overridden in the DoY config file. Here is the default config from `drill-on-yarn-defaults.conf`:
   
   ```
   drill.yarn: {
     ...
     dfs: {
       connection: ""
       app-dir: "/user/drill"
     }
   ```
   
   The code says:
   
   ```java
       String dfsConnection = config.getString(DrillOnYarnConfig.DFS_CONNECTION);
       try {
         if (DoYUtil.isBlank(dfsConnection)) {
           fs = FileSystem.get(yarnConf);
   ```
   
   So, if the `dfs.connection` property is blank, use the one from the YARN config file.
   
   Again, why might there be a different DoY value? Because some users push apps to multiple servers, and the DoY config should be sufficient to do so, without having to have multiple different YARN configs available. (If, in practice, people use only one config, we can remove these DoY configs if not needed. But, let's assume they are needed.)
   
   So, the question is, why did the user see the bug which was reported? Where did the `"hdfs://localhost/"` value come from? **That** is the bug we need to fix.
   
   The answer seems to be that someone used `drill-on-yarn-example.conf` as their config, without inspecting if the *example* values are useful. (This is an *example*, not a *default*.):
   
   ```
   drill.yarn: {
     ...
     dfs: {
       # Connection to the distributed file system. Defaults to work with
       # a single-node Drill on the local machine.
       # Omit this if you want to get the configuration either from the
       # Hadoop config (set with config-dir above) or from the
       # $DRILL_HOME/core-site.xml.
   
       connection: "hdfs://localhost/"
   ```
   
   Why is that being used? The proper "default" file is `drill-on-yarn-override.conf` from `distribution`. But, it looks like the `component.xml` file is missing a line. So, maybe the user renamed the example file to `drill-on-yarn-override.conf`. We need:
   
   ```xml
       <file>
         <source>src/main/resources/drill-on-yarn-override.conf</source>
         <outputDirectory>conf</outputDirectory>
         <fileMode>0640</fileMode>
       </file>
   ```
   
   With the above Maven fix, we don't need to change the code: the code does what it is supposed to do, if given a proper (blank) config entry.
   
   An "extra for experts" fix is to add the updated port number to the example file above.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org