[DISCUSS] Way to maintain failing overflowing config parameters on startup

[Ekaterina Dimitrova <e....@gmail.com>]

Hi everyone,
In Cassandra-17571 I am going to handle overflowing config parameters on
startup - we protect ourselves on converting from bigger to smaller units
by setting MAX_VALUE but this is not obvious on startup and might lead to
misunderstanding and problems. So we need to fail startup and ask for
smaller value from the very beginning.
There are two options to handle the error messaging to the users and I
would like to know from the community which one is preferred:
1) I can add utility methods to validate that the provided value will not
overflow in the smallest allowed unit for the respective parameters and do
the checking in the Database Descriptor. The plus is we can say exactly
which parameter is not OK, the downside is maintainability because someone
might add new parameters and forget to add validations. Then the user will
fall back to assigning MAX_VALUE. We already noticed how startup checks and
some setters do not always match.
Also, David is about to push a patch JMX to intercept all Cassandra
exceptions (like the configuration one) and throw Runtime Exception so we
don’t have to worry about that anymore.
2) After a discussion with David and Caleb I prototyped in CASSANDRA-17571
new extended classes to the new types to handle int bounded parameters and
add checks for the mentioned problems in the constructors. This is less
error prone, validation will always happen at least for the general cases
(now if there is some special case to check for something between 0 and 10
for example this will still go in the DatabaseDescriptor) but the issue is
that in the error message we will have only the overflowing value without
the name of the exact parameter. While this type of overflow might not be a
super common mistake(I hope), especially when we document everything, it
might still lead to inconvenience for someone so I wanted to verify this
with the community before going into one or the other direction.

So to be more descriptive, example of produced error in both cases:
1)" parameter1=very-big-value-in-a-bigger-unit cannot be more than
MAX_VALUE in the-smallest-unit"
2) "very-big-value-in-a-bigger-unit cannot be more than MAX_VALUE in
the-smallest-unit"

I think that if there is the same value more than once then the smallest
unit will guide us which one was the problem. Also, actually you will at
once check all of those same ones instead of failing on different
parameters after every correction on startup.

Let me know what you think. If we go for option 2 I need to migrate
parameters to the prototyped extended classes before we release as that
will mean different types for affected parameters in Config class. If we
have a consensus, I can finish the rest of the work on Monday.

Looking forward to your feedback.
Best regards,
Ekaterina

Re: [DISCUSS] Way to maintain failing overflowing config parameters on startup

[Ekaterina Dimitrova <e....@gmail.com>]

Please ignore my previous email, ZJeremiah pointed I didn’t see snakeyaml
will show the name of the problem parameter, then in the constructor we
provide the problem… that will be also in the trace. Seems I missed that
:-(
I will finish the extended classes approach. Excuse me for the noise,
trying to do the right thing here…

On Fri, 22 Apr 2022 at 10:14, Ekaterina Dimitrova <e....@gmail.com>
wrote:

> Hi everyone,
> In Cassandra-17571 I am going to handle overflowing config parameters on
> startup - we protect ourselves on converting from bigger to smaller units
> by setting MAX_VALUE but this is not obvious on startup and might lead to
> misunderstanding and problems. So we need to fail startup and ask for
> smaller value from the very beginning.
> There are two options to handle the error messaging to the users and I
> would like to know from the community which one is preferred:
> 1) I can add utility methods to validate that the provided value will not
> overflow in the smallest allowed unit for the respective parameters and do
> the checking in the Database Descriptor. The plus is we can say exactly
> which parameter is not OK, the downside is maintainability because someone
> might add new parameters and forget to add validations. Then the user will
> fall back to assigning MAX_VALUE. We already noticed how startup checks and
> some setters do not always match.
> Also, David is about to push a patch JMX to intercept all Cassandra
> exceptions (like the configuration one) and throw Runtime Exception so we
> don’t have to worry about that anymore.
> 2) After a discussion with David and Caleb I prototyped in CASSANDRA-17571
> new extended classes to the new types to handle int bounded parameters and
> add checks for the mentioned problems in the constructors. This is less
> error prone, validation will always happen at least for the general cases
> (now if there is some special case to check for something between 0 and 10
> for example this will still go in the DatabaseDescriptor) but the issue is
> that in the error message we will have only the overflowing value without
> the name of the exact parameter. While this type of overflow might not be a
> super common mistake(I hope), especially when we document everything, it
> might still lead to inconvenience for someone so I wanted to verify this
> with the community before going into one or the other direction.
>
> So to be more descriptive, example of produced error in both cases:
> 1)" parameter1=very-big-value-in-a-bigger-unit cannot be more than
> MAX_VALUE in the-smallest-unit"
> 2) "very-big-value-in-a-bigger-unit cannot be more than MAX_VALUE in
> the-smallest-unit"
>
> I think that if there is the same value more than once then the smallest
> unit will guide us which one was the problem. Also, actually you will at
> once check all of those same ones instead of failing on different
> parameters after every correction on startup.
>
> Let me know what you think. If we go for option 2 I need to migrate
> parameters to the prototyped extended classes before we release as that
> will mean different types for affected parameters in Config class. If we
> have a consensus, I can finish the rest of the work on Monday.
>
> Looking forward to your feedback.
> Best regards,
> Ekaterina
>