[jira] [Assigned] (IGNITE-19152) Named list support in local file configuration is broken.

["Aleksandr (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/IGNITE-19152?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Aleksandr reassigned IGNITE-19152:
----------------------------------

    Assignee: Aleksandr

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