[
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)
