[
https://issues.apache.org/jira/browse/CASSANDRA-15234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17150218#comment-17150218
]
Benjamin Lerer edited comment on CASSANDRA-15234 at 7/2/20, 12:01 PM:
----------------------------------------------------------------------
Some comments on the current patch:
* I would prefer to avoid reading twice the configuration file if possible. It
should be possible to replace {{YamlConfigurationLoader.readStorageConfig}} and
{{isConfigFileValid}} by some checks in {{getProperty}} or by using the
specific converters. As a side note: {{isConfigFileValid}} checks twice for the
same condition on each of the {{if}} conditions.
* The patch can be simplified by making {{Converter}} an {{enum}}. It will
allow to use the enum value directly into the {{Replaces}} annotation. Doing so
will remove the need to use reflection to instantiate the conveters and the use
of a cache to avoid multiple instantiation.
* The change in {{ConfigurationException}} are from some experimentations and
should be removed.
* I would as discussed previously defer the date of removal and remove
{{scheduledRemoveBy}} from the {{Replaces}} annotation.
* It would be nice to have more javadoc on the new code.
was (Author: blerer):
Some high leve comments:
* I would prefer to avoid reading twice the configuration file if possible. It
should be possible to replace {{YamlConfigurationLoader.readStorageConfig}} and
{{isConfigFileValid}} by some checks in {{getProperty}} or by using the
specific converters. As a side note: {{isConfigFileValid}} checks twice for the
same condition on each of the {{if}} conditions.
* The patch can be simplified by making {{Converter}} an {{enum}}. It will
allow to use the enum value directly into the {{Replaces}} annotation. Doing so
will remove the need to use reflection to instantiate the conveters and the use
of a cache to avoid multiple instantiation.
* The change in {{ConfigurationException}} are from some experimentations and
should be removed.
* I would as discussed previously defer the date of removal and remove
{{scheduledRemoveBy}} from the {{Replaces}} annotation.
* It would be nice to have more javadoc on the new code.
> Standardise config and JVM parameters
> -------------------------------------
>
> Key: CASSANDRA-15234
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15234
> Project: Cassandra
> Issue Type: Bug
> Components: Local/Config
> Reporter: Benedict Elliott Smith
> Assignee: Ekaterina Dimitrova
> Priority: Normal
> Fix For: 4.0-alpha
>
> Attachments: CASSANDRA-15234-3-DTests-JAVA8.txt
>
>
> We have a bunch of inconsistent names and config patterns in the codebase,
> both from the yams and JVM properties. It would be nice to standardise the
> naming (such as otc_ vs internode_) as well as the provision of values with
> units - while maintaining perpetual backwards compatibility with the old
> parameter names, of course.
> For temporal units, I would propose parsing strings with suffixes of:
> {{code}}
> u|micros(econds?)?
> ms|millis(econds?)?
> s(econds?)?
> m(inutes?)?
> h(ours?)?
> d(ays?)?
> mo(nths?)?
> {{code}}
> For rate units, I would propose parsing any of the standard {{B/s, KiB/s,
> MiB/s, GiB/s, TiB/s}}.
> Perhaps for avoiding ambiguity we could not accept bauds {{bs, Mbps}} or
> powers of 1000 such as {{KB/s}}, given these are regularly used for either
> their old or new definition e.g. {{KiB/s}}, or we could support them and
> simply log the value in bytes/s.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
