[
https://issues.apache.org/jira/browse/CASSANDRA-17431?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17514983#comment-17514983
]
Ekaterina Dimitrova commented on CASSANDRA-17431:
-------------------------------------------------
So the patch can be found
[here|https://github.com/ekaterinadimitrova2/cassandra/pull/194], respective
properties were added in the ccm dictionary
[here|https://github.com/ekaterinadimitrova2/ccm/compare/master-master...ekaterinadimitrova2:17431-trunk]
in order to satisfy the backward compatibility.
I tried to split into different commits different changes and a few fixes I
did, I had to revert one down the way, please, ignore it.
I left PR commits not squashed so that reviewers can check also individual
commits, to be maybe easier and shorter. Let me know if you prefer everything
squashed.
* As I mentioned already, I didn't touch phi_convict_threshold and
meltable_cleanup_threshold.
* I pinged the author about _repair_request_timeout_in_ms,_ it seems that one
is not needed so he will open a ticket to remove it actually. __
* While working on _block_for_peers_timeout_in_secs_ I realized I have a bug
in MILLIS_CUSTOM_DURATION, -1 should mean null, not 0 which would change the
meaning. This led me also to a few more checks to be added in the
DatabaseDescriptor. While we don't advertise that negative numbers except -1
can be used, we weren't really prohibiting it so I am throwing an exception now
in the setter. Please check if you think this is valid way to handle this and I
am not breaking anything. I added one more unit test to cover this special
case. The idea for the null usage came from [~dcapwell], also one test added
in the YAMLConfigurationLoaderTest is his.
* I ended up not transferring block_for_peers_timeout_in_secs as I noticed
that it can handle even negative numbers which are not rejecter internally by
TimeUnit so I am hesitant to touch this advanced parameter. I will probably
discuss it on the mailing list which I plan to hit after this patch to confirm
with people all special cases we have to validate things before the release.
* While, currently it is not a problem with the current config, we shouldn't
be assigning 0ms in case a property is null but keep it null. This led me to a
change in the MILLIS_DOUBLE_DURATION in regards to the NaN handling for
backward compatibility. While on that I removed the handling of 0 which was
supposed to be accepted without unit too. I remember we were discussing that 0
with a unit might be weird so I can return it back if we think it is better to
be there. This led to broken Paxos tests where 0 without a unit was used. I
fixed those to have a unit but if we prefer really the old way, I will revert
the change.
* I am not 100% sure whether there might be some issue with the new Converter
NEGATIVE_SECONDS_DURATION for validation_preview_purge_head_start_in_sec. I
didn't break any tests but that is not a guarantee as we know... Any
check/advice/suggestion/confirmation will be highly appreciated.
* native_transport_receive_queue_capacity_in_bytes - as I left a comment also
in the code (in order not to forget to mention it during review :) ), this one
is not guarded and mentioned whether it can be negative, etc anywhere. I
definitely have to confirm it with the author/community before doing anything.
[Here|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1487/workflows/ae03b64c-1300-4ed8-a324-4b1ccd176d47]
is a preliminary J8 run, I will run also J11 when we are done with the review.
I was running them down the road. So the only failures that are not already
presented on trunk and which don't have tickets are the Paxos tests failures
and the usage of 0 I mentioned. I pushed commit to fix them after the CI run.
[~dcapwell], [~maedhroz] , I think this is ready for review and discussions
around the corner cases, thank you!
> Move the rest of the Config parameters to the new Config framework
> ------------------------------------------------------------------
>
> Key: CASSANDRA-17431
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17431
> Project: Cassandra
> Issue Type: Task
> Components: Local/Config
> Reporter: Ekaterina Dimitrova
> Assignee: Ekaterina Dimitrova
> Priority: Normal
> Fix For: 4.x
>
>
> Migrate also all Config parameters that are in Config, but not presented in
> cassandra.yaml to the new config framework. Not presented in the yaml as they
> are considered advanced only for advanced users.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
