[
https://issues.apache.org/jira/browse/IGNITE-19152?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mikhail Pochatkin updated IGNITE-19152:
---------------------------------------
Epic Link: IGNITE-19280
> Named list support in local file configuration is broken.
> ----------------------------------------------------------
>
> Key: IGNITE-19152
> URL: https://issues.apache.org/jira/browse/IGNITE-19152
> Project: Ignite
> Issue Type: Bug
> Reporter: Mirza Aliev
> Assignee: Aleksandr
> Priority: Major
> Labels: ignite-3
> Time Spent: 10m
> Remaining Estimate: 0h
>
> After IGNITE-18581 we have started to store local configuration in local
> config file, instead of vault.
> The current flow with the saving configuration to a file has a bug. In the
> method {{LocalFileConfigurationStorage#write}} we call
> {{LocalFileConfigurationStorage#saveValues}} to save configuration fields to
> a file, where we call
> {{{}LocalFileConfigurationStorage#renderHoconString{}}}. Named list value has
> internal id which is {{{}UUID{}}}, but {{com.typesafe}} do not support
> {{{}UUID{}}}, so the whole process of saving configuration to a file fails
> with
> {noformat}
> Caused by: com.typesafe.config.ConfigException$BugOrBroken: bug in method
> caller: not valid to create ConfigValue from:
> 489e16e8-3123-44a3-b27d-6e410863eb24
> at
> app//com.typesafe.config.impl.ConfigImpl.fromAnyRef(ConfigImpl.java:282)
> at
> app//com.typesafe.config.impl.PropertiesParser.fromPathMap(PropertiesParser.java:165)
> at
> app//com.typesafe.config.impl.PropertiesParser.fromPathMap(PropertiesParser.java:95)
> at
> app//com.typesafe.config.impl.ConfigImpl.fromAnyRef(ConfigImpl.java:265)
> at
> app//com.typesafe.config.impl.ConfigImpl.fromPathMap(ConfigImpl.java:201)
> at
> app//com.typesafe.config.ConfigFactory.parseMap(ConfigFactory.java:1225)
> at
> app//com.typesafe.config.ConfigFactory.parseMap(ConfigFactory.java:1236)
> at
> app//org.apache.ignite.internal.configuration.storage.LocalFileConfigurationStorage.renderHoconString(LocalFileConfigurationStorage.java:208)
> at
> app//org.apache.ignite.internal.configuration.storage.LocalFileConfigurationStorage.saveValues(LocalFileConfigurationStorage.java:185)
> at
> app//org.apache.ignite.internal.configuration.storage.LocalFileConfigurationStorage.write(LocalFileConfigurationStorage.java:138)
> at
> app//org.apache.ignite.internal.configuration.ConfigurationChanger.changeInternally0(ConfigurationChanger.java:606)
> at
> app//org.apache.ignite.internal.configuration.ConfigurationChanger.lambda$changeInternally$1(ConfigurationChanger.java:541)
> {noformat}
> h3. More details
> The problem is trickier than it may seem.
> Configuration storages receive data in "flat" data format, meaning that the
> entire tree is converted into a list of pairs:
> {code:java}
> [{ "dot-separated key string", "serializable value" }]{code}
> LocalFileConfigurationStorage interprets keys as literal paths in HOCON
> representation, which is simply not correct. These keys and values also have
> meta-information, associated with them, such as:
> * order of elements in named list configuration
> * internal ids for named list elements
> To see, what's exactly in there, you may refer to the
> {{{}org.apache.ignite.internal.configuration.tree.NamedListNodeTest{}}}. It
> has everything laid out explicitly.
> h3. Proposed fix
> Well, the ideal approach would be rendering the configuration more or less
> the same way, as we do it for REST.
> It means calling {{ConfigurationUtil#fillFromPrefixMap}} for every local root.
> Local roots can be retrieved using {{{}ConfigurationModule{}}}, by reading
> them all from the class path.
> Resulting nodes are converted to maps using {{{}ConverterToMapVisitor{}}}.
> Then maps are converted to HOCON using its own API.
> There are several hidden problems here.
> * {-}we must check, that HOCON preserves order of keys{-}, and that we use
> linked hash maps in {{fillFromPrefixMap}}
> EDIT: HOCON sorts keys alphabetically. Ok
> * {{ConverterToMapVisitor}} does not expect null nodes, because it always
> works with "full" trees. Fixing it would require some fine-tuning, otherwise
> one may end up with a bunch of empty nodes in the config file, which is bad
> * {{ConverterToMapVisitor}} uses array syntax for named lists. You can see
> it in action in {{{}HoconConverterTest{}}}.
> Yes, there are two ways of representing named lists in the system. We should
> make rendering mode configurable, because local configuration, at the moment,
> only needs basic tree representation (for node attributes)
> We should also add tests for most of these improvements. First of all, to
> {{{}HoconConverterTest{}}}.
> h3. Misc
> Another extremely uncertain thing is the way we handle default values. This
> may be a topic for another issue, or maybe not.
> For example, if user explicitly configure network port to 47500, we will fail
> to save it into the file, because it matches default and we ignore everything
> that has default value. We *need* tests for this.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
