So my suggestion from what I hear - reject anything below -1. - -1 points to the default. - >=0 being valid. - Done for both startup and the setters. - Done only on trunk for now and I will add comment in Config instead of NEWS.txt due to their hidden nature. So my understanding is that 0 and 1 will do kind of the same but it is just not right not to accept 0 and point it to the default value which -1 does. Please correct me if I am wrong. Also, we anyway reject negatives in trunk with the new types except for some special cases where things like -1 were legit before for some parameters. (More info in the Converters) Those will still work with the old yaml and being translated to null with the new types. Documented
On Mon, 4 Apr 2022 at 16:54, David Capwell <dcapw...@apple.com> wrote: > I am good with making >= 0 valid in both setters and config parsing, > negatives should error or produce defaults > > > On Apr 4, 2022, at 10:45 AM, Caleb Rackliffe <calebrackli...@gmail.com> > wrote: > > I'm for making >= 0 valid in both the setters and on startup. In the > setters, I'm fine with either translating negative values to the default > calculated value based on heap size or simply rejecting negative values. If > we really want to override that value, we had better have a really good > idea of why... > > Given this has always been an advanced feature, I'm neutral on adding > anything to NEWS.txt. > > On Mon, Apr 4, 2022 at 8:46 AM Ekaterina Dimitrova <e.dimitr...@gmail.com> > wrote: > >> Hi everyone, >> While working on CASSANDRA-17431 to move the advanced Config parameters >> we don't advertise in cassandra.yaml to the new types created in >> CASSANDRA-15234, we ran into some concerns around >> native_transport_max_concurrent_requests_in_bytes >> and native_transport_max_concurrent_requests_in_bytes_per_ip. >> While on startup if those parameters are set to something <= 0, they will >> get a default value, their setters are not guarded in the same way, we just >> set any long value. >> I checked and this is actually something going on all currently supported >> Cassandra versions. From there Caleb raised the question that 0 actually >> can be a valid value and the check needs to cover possibly only < 0. >> As these are advanced and non-documented parameters, I thought it is >> valuable to confirm the course of action here. Especially, if we are >> willing to do a change to all released versions. >> So the proposal is to: >> 1) change the check on startup from <=0 to <0 and make 0 a valid value, >> for all versions? >> 2) Add the same checks&assignment to the respective setters >> 3) Update NEWS.txt explicitly (this part is a bit weird to me as we keep >> it not advertised in cassandra.yaml but we want whoever uses it to know >> about such changes...) >> >> Looking forward to your feedback, thanks in advance. >> >> Best regards, >> Ekaterina >> > >
