[
https://issues.apache.org/jira/browse/CASSANDRA-15234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17145118#comment-17145118
]
David Capwell commented on CASSANDRA-15234:
-------------------------------------------
This is a large patch, so will take a few attempts to review; below is my first
pass (mostly skim).
* Config. otc_coalescing_window_us_default was not renamed but the other otc*
configs were.
https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-15234-new#diff-b66584c9ce7b64019b5db5a531deeda1R429
* block_for_peers_timeout_in_secs and block_for_peers_in_remote_dcs are exposed
in yaml, the comment says "No need of unit conversion as this parameter is not
exposed in the yaml file", can you explain why this comment?
*
https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-15234-new#diff-76c34368f112a0a4e6da5b22c8b50300R353-R376.
enums can use "abstract" methods, so you can do that rather than use a default
which throws
* in database descriptor I see you use toString to compare, why not convert to
the expected unit?
https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-15234-new#diff-a8a9935b164cd23da473fd45784fd1ddR381.
this could be {code}if (conf.commitlog_sync_period.toMillis() != 0){code}.
This is true for all examples I see in this method
* we should add docs for Replaces and ReplacesList (also calling out that you
shouldn't use ReplacesList directly)
* for the new types, would be good to have a property test which validates that
{code}type(value, unit).equals(type.parse(type(value, unit).toString())){code}.
* LoadOldYAMLBackwardCompatibilityTest and ParseAndConvertUnitsTest look to be
copy/paste. junit supports inheritance so could remove the copy based by
having LoadOldYAMLBackwardCompatibilityTest extend ParseAndConvertUnitsTest but
use the other config.
Overall I am good with this patch, need to do a closer review of the details
still.
> 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]
