You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Ekaterina Dimitrova (Jira)" <ji...@apache.org> on 2021/11/22 03:59:00 UTC
[jira] [Comment Edited] (CASSANDRA-15234) Standardise config and JVM parameters
[ https://issues.apache.org/jira/browse/CASSANDRA-15234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17447181#comment-17447181 ]
Ekaterina Dimitrova edited comment on CASSANDRA-15234 at 11/22/21, 3:58 AM:
----------------------------------------------------------------------------
[~dcapwell],[~blerer], submitting for new round of review.
I have some issue that I can't always trigger to get the newest version I push to ccm. Still it seems Jenkins has more success than CircleCI.
Until I figure it out, these are the patches for initial round of review.
[trunk|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-15234-take2?expand=1] | [dtest|https://github.com/ekaterinadimitrova2/cassandra-dtest/pull/new/CASSANDRA-15234-take2] | [ccm|https://github.com/ekaterinadimitrova2/ccm/pull/new/CASSANDRA-15234]
I cleaned all tests without full Jenkins confirmation of the dtests yet due to the ccm issue mentioned.
Also, this patch latest rebase was also a week ago and I will rebase again when I have to do next round of review changes and I fix ccm as otherwise on every rebase something new appears as trunk is quite alive and the config is all over the place. Let me first stabilize CI and we can agree on the patch, then I will clean this.
Now a few important comments:
* ASCII doc migration is still not done but I will open a ticket to add docs for us for adding new config and docs for our users around the new and old types and names.
* DTests were not updated to the new names and format as this will take forever. Suggestion - leave the old ones and add new tests with new ones. We can also open a follow up ticket but now this exercises the backward compatibility which is good as this is also how I figured out one more change needed for CCM.
* CCM was updated to be able to handle the backward compatibility and Cassandra behavior.
* I suggest not to overwhelm the patch with the reorganization of cassandra.yaml I suggested earlier on this ticket. I will pull it in a separate one and I can trigger a new discuss thread on the mailing list to confirm that.
* Converter was not changed to an enum because we need generics support for that and [JEP301|http://openjdk.java.net/jeps/301] was withdrawn.
* Intentionally left the commits after the old patch not squashed to provide some input and hopefully make the review a bit more easy. [~blerer] and [~dcapwell] , please, let me know if you want them in a different way for the review.
* Now when 4.0 was released I had to add also Virtual Table backward compatibility and the easiest way was just to have the old names and types and the new ones both listed in the Settings table.
* commitlog_sync_batch_in_ms was left for another ticket as the in-jvm upgrade tests will need work to make it possible to support it due to their nature as it seems we have the lower branch DatabaseDescriptor class loaded and it makes checks on trunk and complains if we change to the custom type.
* Any new parameters added post 4.0 in trunk were/will be directly changed to support the new format as they are still not in any release.
* CCM_41_YAML_OPTIONS in ccm should be well documented and people warned that they need to keep it up to date on changes in config. I hope we won't need this to happen a lot after this update but we need to ensure people are aware of it. It seems a bit clumsy but I didn't find a better way to handle ccm at this point. Open for ideas.
* I can think of adding some additional test for the virtual table.
* I also suggest we add a warning to the users when they start Cassandra that if they add more than once a property (even if it is the old name), the latest occurrence from the yaml file will appear. I would say we add also to the docs and we don't complicate to add checks whether we have or not more occurrences and on occurrences adding it as it seems a bit too much overhead on startup. WDYT?
* There were deprecation warnings emitted more than once. I fixed this. This fix should be also ported to 4.0 where we had to make small port of the name change because of two parameters issues. (FTR - CASSANDRA-17141)
was (Author: e.dimitrova):
I have some issue that I can't always trigger to get the newest version I push to ccm. Still it seems Jenkins has more success than CircleCI.
Until I figure it out, these are the patches for initial round of review.
[trunk|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:CASSANDRA-15234-take2?expand=1] | [dtest|https://github.com/ekaterinadimitrova2/cassandra-dtest/pull/new/CASSANDRA-15234-take2] | [ccm|https://github.com/ekaterinadimitrova2/ccm/pull/new/CASSANDRA-15234]
I cleaned all tests without full Jenkins confirmation of the dtests yet due to the ccm issue mentioned.
Also, this patch latest rebase was also a week ago and I will rebase again when I have to do next round of review changes and I fix ccm as otherwise on every rebase something new appears as trunk is quite alive and the config is all over the place. Let me first stabilize CI and we can agree on the patch, then I will clean this.
Now a few important comments:
* ASCII doc migration is still not done but I will open a ticket to add docs for us for adding new config and docs for our users around the new and old types and names.
* DTests were not updated to the new names and format as this will take forever. Suggestion - leave the old ones and add new tests with new ones. We can also open a follow up ticket but now this exercises the backward compatibility which is good as this is also how I figured out one more change needed for CCM.
* CCM was updated to be able to handle the backward compatibility and Cassandra behavior.
* I suggest not to overwhelm the patch with the reorganization of cassandra.yaml I suggested earlier on this ticket. I will pull it in a separate one and I can trigger a new discuss thread on the mailing list to confirm that.
* Converter was not changed to an enum because we need generics support for that and [JEP301|http://openjdk.java.net/jeps/301] was withdrawn.
* Intentionally left the commits after the old patch not squashed to provide some input and hopefully make the review a bit more easy. [~blerer] and [~dcapwell] , please, let me know if you want them in a different way for the review.
* Now when 4.0 was released I had to add also Virtual Table backward compatibility and the easiest way was just to have the old names and types and the new ones both listed in the Settings table.
* commitlog_sync_batch_in_ms was left for another ticket as the in-jvm upgrade tests will need work to make it possible to support it due to their nature as it seems we have the lower branch DatabaseDescriptor class loaded and it makes checks on trunk and complains if we change to the custom type.
* Any new parameters added post 4.0 in trunk were/will be directly changed to support the new format as they are still not in any release.
* CCM_41_YAML_OPTIONS in ccm should be well documented and people warned that they need to keep it up to date on changes in config. I hope we won't need this to happen a lot after this update but we need to ensure people are aware of it. It seems a bit clumsy but I didn't find a better way to handle ccm at this point. Open for ideas.
* I can think of adding some additional test for the virtual table.
* I also suggest we add a warning to the users when they start Cassandra that if they add more than once a property (even if it is the old name), the latest occurrence from the yaml file will appear. I would say we add also to the docs and we don't complicate to add checks whether we have or not more occurrences and on occurrences adding it as it seems a bit too much overhead on startup. WDYT?
* There were deprecation warnings emitted more than once. I fixed this. This fix should be also ported to 4.0 where we had to make small port of the name change because of two parameters issues. (FTR - CASSANDRA-17141)
> 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: 5.x
>
> 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.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org