[
https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17538537#comment-17538537
]
Ekaterina Dimitrova commented on CASSANDRA-17571:
-------------------------------------------------
I addressed review comments from [~adelapena]. New CI run
[here|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra?branch=review-2&filter=all].
There is one issue with DTest which is kind of weird and I will have to
investigate it but other community members confirmed they've seen it too so it
is not this patch. Might be even environmental. Also, one of the CQLSH run
tests failed because of timeout during test docker image being pulled so it was
not the patch.
Also, for the record, based on offline discussion, we limited the conversions
in the nested classes to only to the base internal min unit. This actually led
me to discover a new parameter added which was converting to other units
internally and probably relying on cassandra.yaml for the precision (there was
a note on allowed units in cassandra.yaml without actually ensuring a lower
unit was not provided by the user). I think this is a bug and I fixed it by
making auto_snapshot_ttl internally in seconds as we have
SNAPSHOT_MIN_ALLOWED_TTL_SECONDS.
This led to this
[change|https://github.com/ekaterinadimitrova2/cassandra/pull/199/files#diff-20db300621aef19985cb1752835ab00d37ed18a4abc23e3b67c769633d41a9a6L61].
Please let me know if you agree with this and I will update also
cassandra.yaml. I think the other option based on the yaml is to do it minutes
but then I guess we need to change also SNAPSHOT_MIN_ALLOWED_TTL_SECONDS and we
are already in code freeze so I think setting min unit seconds is a good
compromise. Considering we didn't announce in the yaml accepted units less than
minutes for that property I do not consider this change a breaking one. More
like preventive and fixing internally a possible precision issue/bug.
The story of this property is that it was added during Google Summer of Code
with a WIP version of my DurationSpec class. When it came the time for
CASSANDRA-15234 we changed the name of the class. Then I asked the author to
check the property if there are any concerns and it seems he missed the point
for precision described in the docs. So long story short - thank you
[~adelapena] for suggesting to tighten even more these classes! It is
definitely worth it as a preventive action!
> Config upper bound should be handled earlier
> --------------------------------------------
>
> Key: CASSANDRA-17571
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17571
> Project: Cassandra
> Issue Type: Bug
> Components: Local/Config
> Reporter: Ekaterina Dimitrova
> Assignee: Ekaterina Dimitrova
> Priority: Normal
> Fix For: 4.1-beta, 4.x
>
>
> Config upper bound should be handled on startup/config setup and not during
> conversion
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
