[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17540301#comment-17540301 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Ticket for the flaky tests in TopPartitionsTest opened CASSANDRA-17649 > 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-alpha, 4.1 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17540274#comment-17540274 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - The patch was committed after checking offline with [~maedhroz] and [~adelapena] for final +1. Special thanks to [~adelapena] for helping me to verify CI and double checking the rebase which had some conflicts. There were a few unknown to me failures but we verified that they are not new by looping both with and without the patch: * 4.1 - _basicRangeTombstones_ - without the patch failed a few times here - [https://app.circleci.com/pipelines/github/adelapena/cassandra/1585/workflows/70aa2792-56ca-49e3-8fc7-749270041edc] with the patch I found it once by looking at the logs of the failing containers as UI was not showing which are the failing tests - https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra?branch=17571-more-tests=all * trunk - [handleCorruptionOfLargeMessageFrame-compression|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1638/workflows/eb0712f6-d19e-4b0a-b0df-30e5107f880c/jobs/11342/tests#failed-test-0] [testNegativeEnvelopeBodySize-compression|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1638/workflows/eb0712f6-d19e-4b0a-b0df-30e5107f880c/jobs/11342/tests#failed-test-1] [testUnrecoverableMessageDecodingErrors-compression|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1638/workflows/eb0712f6-d19e-4b0a-b0df-30e5107f880c/jobs/11342/tests#failed-test-2] [testRecoverableEnvelopeDecodingErrors-compression|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1638/workflows/eb0712f6-d19e-4b0a-b0df-30e5107f880c/jobs/11342/tests#failed-test-3] all those were proved as failing as part of CASSANDRA-16677. Multiplexer run - [https://app.circleci.com/pipelines/github/adelapena/cassandra/1405/workflows/88fb8e4f-7a3f-42f9-a6db-5d14a652b54e/jobs/14141/tests] > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17540201#comment-17540201 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - [trunk|https://github.com/ekaterinadimitrova2/cassandra/pull/new/17571-trunk-final] patch, [CI|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra?branch=17571-trunk-final=all] started If everything is fine, I will add CHANGES.txt entry, add [~adelapena] as co-author and also add to the entry that we fixed two properties - auto_snapshot_ttl and paxos_purge_grace_period > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17540196#comment-17540196 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Rebased [here|https://github.com/ekaterinadimitrova2/cassandra/pull/new/17571-final] in a new branch to preserve the unsquashed version for reference. (there were unfortunately merge conflicts after the exception handling patch committed yesterday but I think things are fine, at least unit tests Guardrails, setGet* and config are passing locally). The move of the static conversion methods led to IntKibibytesBoundToBytes which led to the need of change in the DatabaseDescriptor [here|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:17571-final?expand=1#diff-054af65b8d690b0fddc3e0a4ef05a80d8f1d6689b4f77912795fec019200666cR551] and [here|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:17571-final?expand=1#diff-054af65b8d690b0fddc3e0a4ef05a80d8f1d6689b4f77912795fec019200666cR3706] which should be fine as Integer.MAX_VALUE cannot be divided by 1024 to an integer so we don't need it. I also removed one of the non-negative validations in the constructor which doesn't need it because that is ensured already by the pattern matching. I added a test for that too [here|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:17571-final?expand=1#diff-b02560e1a47ddce2e07855a3aaf2931b1987e17d75ff99d01d44fedcaf2d83b1R87] CI running [here|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra?branch=17571-final=all] trunk comes in 10 minutes I hope as I don't expect the cherry-picked commit to have many conflicts there. > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17539588#comment-17539588 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Rebased and pushed the latest round of feedback being addressed. > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17539109#comment-17539109 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Addressed your feedback and fixed a few nits I noticed. There are a few warnings in the new types tests around equals but I don't think we should do any changes there. Maybe those assertions can be removed, they prove 5s are 5s no matter what. :) > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[
https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17538911#comment-17538911
]
Ekaterina Dimitrova commented on CASSANDRA-17571:
-
{quote}bq. the conversion methods in {{DataStorageSpec}} [are still
present|https://github.com/ekaterinadimitrova2/cassandra/blob/f2d56d54bf62f18e8daea7736639119c3fe1b89a/src/java/org/apache/cassandra/config/DataStorageSpec.java#L137-L189],
and the leaf classes override them.
{quote}
I knew it this green CI hides something :( Totally not meant to keep them. I
will work it out, thanks.
The commit looks good to me
> 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: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[
https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17538776#comment-17538776
]
Andres de la Peña commented on CASSANDRA-17571:
---
[~e.dimitrova] I see that the conversion methods in {{DataStorageSpec}} [are
still
present|https://github.com/ekaterinadimitrova2/cassandra/blob/f2d56d54bf62f18e8daea7736639119c3fe1b89a/src/java/org/apache/cassandra/config/DataStorageSpec.java#L137-L189],
and the leaf classes override them.
Also, if we are going to move the conversion methods to the leaf classes, we
can probably get rid of the {{to*AsInt}} methods, and instead have, for example:
* A {{DataStorageSpec.LongMebibytesBound#toMebibytes}} that returns {{long}}
* A {{DataStorageSpec.IntMebibytesBound#toMebibytes}} that returns {{{}int{}}}.
That would be possible because we don't have a common inherited
{{DataStorageSpec#toMebibytes}} anymore. I gave it a quick try in [this
commit|https://github.com/adelapena/cassandra/commit/a7485d88a434dc321a60a006d4f36b294a139d49],
which is just a sketch to show how it would look like.
> 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: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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=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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17537881#comment-17537881 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Thanks, I just addressed the review comments and [ran CI|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra?branch=review=all] on 4.1. 2 known failures and 2 unit tests that I have missed to update. Added to the PR a fix now. A few things to mention here: * I also updated the docs plus added min unit in cassandra.yaml for the properties where it made sense. What I mean - if a property default value is in B in the yaml, it is obvious we can set all units, so it didn't make sense to me to pollute the yaml for those properties. * Worth to mention here that now all three main classes (DataRateSpec, DurationSpec, DataStorageSpec) are abstract and people should use the extended classes which are now tested. I added a test to validate Config types, that people didn't use as type abstract class with constructor of a nested class. This will compile fine and if no value is set in cassandra.yaml, everything will be fine, BUT if someone adds a value in the yaml - then Cassandra won't start because SnakeYAML will want to use abstract class constructor. [The test|https://github.com/ekaterinadimitrova2/cassandra/pull/199/files#diff-392bbe6df967db1b9b2c70cb97bbd84779fa847b11573ed11ca601578ebf2936R48-R59] I added checks that no parameter has as a type any of the three abstract classes to prevent us from future bugs. In that sense, paxos_purge_grace_period is fixed now as the author has done the mistake I mentioned originally. (using the main class and because the main classes were not abstract this was even worse as de facto the smallest Duration unit was set to nanoseconds). > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17536794#comment-17536794 ] Caleb Rackliffe commented on CASSANDRA-17571: - +1 > 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.x, 4.1-beta > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17535879#comment-17535879 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - [4.1 patch|https://github.com/apache/cassandra/compare/cassandra-4.1...ekaterinadimitrova2:17571-squashed]| [CI|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra?branch=17571-squashed=all] I will update the docs when we confirm the details during review. > 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.x, 4.1-beta > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17535063#comment-17535063 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - For the record, [~adelapena] suggested nested classes as we might need also other types in the future and that refactoring will help us to maintain the types easily. Also, the code shrinks nicely and all upper bound validations will be in the constructors. I will post the final version for review later today. > 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.x, 4.1-beta > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17529705#comment-17529705 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - WIP: [https://github.com/ekaterinadimitrova2/cassandra/pull/new/17571-trunk-rebased] Intermediate CI runs: [https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1569/workflows/f67be620-844b-4151-9cd9-e7b16b9e0578] - the DD test was fixed after that but didn't rerun the whole suite only for it... [https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1570/workflows/a8f6c673-a5e6-4986-8077-e2955d6ab949] –> I probably need to loop the test before and after the patch, known to be flaky but just to ensure frequency and errors match [https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1571/workflows/90ef96bd-daa8-4a65-8915-d3ca746bd10d] - known failures So the biggest part is done. I need to verify that ConnectionTest, I didn't contribute in a way for more flakiness but I doubt it. Other ToDos - there are two former int parameters unreleased that were directly switched to the new types, I have them ready locally, need to double check and commit. News.txt, CHANGES.txt and docs need update; also JavaDoc. I had in ToDo to move the *AsInt methods from the parent classes to the extensions but I am not sure I will have to. I will check tomorrow morning and push for final review. > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17528792#comment-17528792 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Working to migrate the parameters to the extended classes so we can do upper bound (former int type Config parameters and overflow of big value bigger unit to smaller supported internally) checks in the constructors. Might be not most beautiful but at least we will be more consistent and less error-prone then utility methods in the DD. > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17528475#comment-17528475 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - On the bright side, this is not a regression as we handle the old config input types in Converters(former ints cannot be set long value) so in my humble opinion even if we fix the new config upper bound after the freeze on the 1st, this is not a regression we have here and no API change will be involved. I don't plan too prolong it but if we want more time to shape the solution a bit or for final review - I think we have it. CC [~mck] in case he disagrees with this statement > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17528472#comment-17528472 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - I spent my evening again thinking about this as really those extensions become convoluted but I don't see a better way at this point. Like I can get back validation utility methods in the DD and leave considering better general handling with next version but this is error-prone. Creating a new range max annotation to all new properties in Config is not really easy option as we would need conversions... The best is to do it in the constructors at this point. I will sleep on it and finish tomorrow morning. In case someone has something better in the meantime - I am open to hear it. > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17528372#comment-17528372 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Thanks, looking forward for your feedback. I am on standby ready to incorporate any feedback and move the former int config to the extended classes before the 1st is here. Unfortunately, it will become noisy but it should be quick type change, using the old methods. > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17527572#comment-17527572 ] Caleb Rackliffe commented on CASSANDRA-17571: - [~e.dimitrova] I'll take a look at the prototype today or tomorrow. > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17527229#comment-17527229 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - [~dcapwell] , [~maedhroz], [~mck], [~adelapena] is anyone of you available to confirm the drafted prototype and do review this week or should I ask anyone else from the community if you won't have time? > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17526150#comment-17526150 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Prototype in this [commit |https://github.com/ekaterinadimitrova2/cassandra/commit/1ab9f32ef34402a0f74036d768a22449170052b6] - only a few parameters were migrated for test purposes and to see how it will look like. Also, I will split in separate commits the parameters in groups on migration with attached tests to them and CI to be sure gradually nothing Is missed but I want to confirm that the approach is still what we want. CC [~adelapena] in case he has time to provide input. Currently if people provide the new config with the new format we handle the former int parameters by returning cast value from their getters, but on startup the user might set a bigger long value and think wrongly that one will be used when in practice the Integer.MAX_VALUE will be used. We need just to fail the user they can't set that big value, mimic the behavior of when they provide old value bigger than int. We also limit with these classes that people cannot set anything that will overflow during conversion to the smallest allowed unit instead of setting MAX_VALUE silently. > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-17571) Config upper bound should be handled earlier
[ https://issues.apache.org/jira/browse/CASSANDRA-17571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17525782#comment-17525782 ] Ekaterina Dimitrova commented on CASSANDRA-17571: - Marking as 4.1 block as there was a discussion to add extended classes for Int to handle old int parameters upper bound and changing those in Config will be considered breaking change after a release. CC [~dcapwell] and [~maedhroz] and [~mck] I will push the suggested classes in the next few hours for approval before moving any config to them. > 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 > > > 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
