[ https://issues.apache.org/jira/browse/DRILL-5547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16083206#comment-16083206 ]
ASF GitHub Bot commented on DRILL-5547: --------------------------------------- Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/868#discussion_r126828646 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java --- @@ -48,11 +50,46 @@ * Only one instance of this class exists per drillbit. Options set at the system level affect the entire system and * persist between restarts. */ + +/** + * Drill has two different config systems each with its own namespace.First being the HOCON based boot time config + * system.This is a hierarchical system where the top layers override the bottom ones in the following order + * + * Java System Options + * distrib.conf + * drill-override.conf + * drill-module.conf + * + * These are the options that are set before the drill starts.But once drill starts System or session options can be + * modified using ALTER SYSTEM/SESSION.Even this system provides inheritance sytle in the following order + + * Session options + * System options + * Hardcoded defaults + + * But system/session options have a validator and the validator has a hard coded default value for every option. In + * the current system validators are registered so that system/session options will always have a default value. + * So when a system/session options are not explicitly set or a user system/session option is null the hardcoded + * default was applied since it checks if the option value is null and returns the default set in the validator.But + * the config options set during boot time are never read and honored since there is no linkage between the two + * config systems.It is also evident that there are some places where there is some ad-hoc linkage between the + * two systems.For example, for the code gen compiler, config options are supposed to be read if the system option + * is not null.But as the validator provides the default values config options are never taken into consideration. + * + * The goal of the new system is to link both the systems in such a way that boot-time config options take precedence + * over the hard coded defaults set in the validator.All the options in the option validator i.e.c options from + * Exec constants, planner settings etc., are extracted and put into a new name space called drill.exec.options + * in the .conf file. + * The default values of the validators in the option validator are populated with the values in the boot-config. + * This way the values set in the boot time config system are honored.Any user who wish to change the option + * values in the config should change the options under the name space drill.exec.options + * + * + */ public class SystemOptionManager extends BaseOptionManager implements OptionManager, AutoCloseable { private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(SystemOptionManager.class); - private static final CaseInsensitiveMap<OptionValidator> VALIDATORS; - + private static CaseInsensitiveMap<OptionValidator> VALIDATORS; --- End diff -- Why no longer `final`? Once this map is built, it should never be replaced. (The map itself can be changed; `final` just means that the `VALIDATORS` variable itself can't be changed.) And, see comments below in the method that does the defaults setup. > Drill config options and session options do not work as intended > ---------------------------------------------------------------- > > Key: DRILL-5547 > URL: https://issues.apache.org/jira/browse/DRILL-5547 > Project: Apache Drill > Issue Type: Bug > Components: Server > Affects Versions: 1.10.0 > Reporter: Karthikeyan Manivannan > Assignee: Venkata Jyothsna Donapati > Fix For: Future > > > In Drill, session options should take precedence over config options. But > several of these session options are assigned hard-coded default values when > the option validators are initialized. Because of this config options will > never be read and honored even if the user did not specify the session > option. > ClassCompilerSelector.JAVA_COMPILER_VALIDATOR uses CompilerPolicy.DEFAULT as > the default value. This default value gets into the session options map via > the initialization of validators in SystemOptionManager. > Now any piece of code that tries to check if a session option is set will > never see a null, so it will always use that value and never try to look into > the config options. For example, in the following piece of code from > ClassCompilerSelector (), the policy will never be read from the config file. > policy = CompilerPolicy.valueOf((value != null) ? > value.string_val.toUpperCase() : > config.getString(JAVA_COMPILER_CONFIG).toUpperCase()); -- This message was sent by Atlassian JIRA (v6.4.14#64029)