[
https://issues.apache.org/jira/browse/IGNITE-19152?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ivan Bessonov updated IGNITE-19152:
-----------------------------------
Description:
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{}}}.
was:
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}}
* {{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{}}}.
> 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
> 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{}}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
