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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
