I have checked with Ekaterina, we are ok with what she asked us to take a look at. Other than that, I do not remember I have touched / worked with any other mentioned parameter she elaborated on between 4.0 and 4.1 so I can not comment on that.
On Tue, 5 Apr 2022 at 23:24, Ekaterina Dimitrova <e.dimitr...@gmail.com> wrote: > > ATTENTION PLEASE Below email will be long but I believe you will agree it > deserves attention for good reasons. Thank you in advance for your time and > consideration! > > > Hi everyone, > > I am working on the last batch of config parameters to be transferred to the > new types after CASSANDRA-15234 landed. > > This led to a few questions and concerns and here I am to raise awareness and > one more time to confirm things for the community to ensure there is no > regression when 4.1 is out. > > As the main decisions were taken and implemented two years ago before even > 4.0 beta, I would like to ensure that everyone here is aware that in > CASSANDRA-15234 we have backward compatibility to transfer the old format > (only values, no opportunity to change unit) to the new format (value + unit) > but it falls back to the new types at the end. This is not a feature with a > flag to be disabled. We migrated the parameters to the new types in Config > class and in cassandra.yaml. Also, we explicitly say we don’t support > negative values (old and new yaml config) which was not the case before. We > never advertised the usage of negative values but we also did not prohibit > that which we do now as one more improvement in CASSANDRA-15234. I will add > an explicit note in NEWS.txt to be sure no one was expecting to keep on using > negative values with the old yaml if they were doing it for some unknown > reason, old/new - we prohibit on trunk negative values except special cases. > > As part of the documentation I mentioned the Converter class which serves to > do these conversions between old value and new value - load the old value to > the new Config types. There are special cases which I want to highlight in > case someone knows of any behavior not caught by our CI that might be changed > or broken because of those changes(better safe than sorry): > > The new types DataRateSpec, DurationSpec and DataRateSpec accept only > non-negative values (this will affect both old and new config). Thus as part > of the converters we have the following special cases: > > - MILLIS_DOUBLE_DURATION for commitlog_sync_group_window_in_ms or now already > commitlog_sync_group_window which covers that this was double and even if we > think this was a bug as it is casted later to int here[1], someone might have > been using double and NaN. Disallow less than 0 as we already said for all > types. With the new config types this parameter is stored as long. > > - MILLIS_CUSTOM_DURATION for permissions_update_interval, > roles_update_interval and credentials_update_interval. After CASSANDRA-17431 > those will be updated to - old value of “-1” being translated to null. The > setters will be doing that too. Anything below -1 is prohibited and > considered a bug (both with old and new config). > > - we are adding NEGATIVE_SECONDS_DURATION as part of C17431 to handle > validation_preview_purge_head_start. Anyone using the old yaml and value <0 > will have it translated into 0 seconds. New yaml and format prohibits values > less than 0 and the smallest unit for this parameter is seconds. > > - BYTES_CUSTOM_DATA_STORAGE translates “-1” to “null” and prohibits anything > less than -1 with the old yaml and less than 0 with the new one for > native_transport_max_concurrent_requests_in_bytes_per_ip and > native_transport_max_concurrent_requests_in_bytes after C17341. I had also > separate mail for these two as we have some concerns about them for all C* > versions. > > In C17431 I also decided to leave parameters phi_convict_threshold, > memtable_cleanup_threshold, block_for_peers_timeout_in_secs alone and not to > migrate them to the new Config types because of various reasons explained in > the ticket. Please let me know if you have any particular > suggestions/concerns/thoughts around those. > > DataRateSpec - it was requested to support mebibytes/s, kibibytes/s, bytes/s. > This made things complicated internally with two of the parameters introduced > in 4.0 which were in megabits/s and I had to keep the old behavior and them > being able to still support megabits/s with the old format. As the conversion > between megabits and mebibytes is not really a whole number, I had to store > the DataRateSpec in double and not long as in the other types in order to > ensure accurate precision, etc. We considered this being fine because the > RateLimiter uses double and we still allow only whole numbers to be provided > by the users. Please let me know if you see any problem. A quick unit test > where you can validate how those work is SetGetInterDCStreamThroughputTest. > Important to mention that once people move to the new format, they can’t > assign them anything less than 1MiB/s. > > Regarding DataRateSpec - the two new parameters > (entire_sstable_stream_throughput_outbound_megabits_per_sec and > entire_sstable_inter_dc_stream_throughput_outbound_megabits_per_sec) still > not being in release were changed to be in MiB/s. Checked with Francisoco in > Slack - that should be fine. > > I would also like to ask Stefan and Paulo to check if they do not agree with > anything in the latest version of DurationSpec as they have introduced the > initial version of Duration in the codebase which evolved into DurationSpec > in C15234. Please let me know if there is anything else aside from the tests > you introduced that I might be missing and it changes your intentions. > Already sent a msg in Slack in case they miss the email. > > > Also, it was discussed on the ticket but I wanted to add it again for > consideration - virtual tables currently show both the old names and format > and new names and format. There are only three parameters which change only > type but not names. Those will be listed only with the new format value. This > was not considered as enough reason to bump to 5.0 because of breaking change > but I wanted one more time everyone to be aware of that agreement. > > > In addition, I will add one more note to NEWS.txt for people to raise > awareness to check carefully their config on upgrade that there are no > breaking changes. We already mention the new option and the backward > compatibility but I think it won’t hurt to stress more on the part we > prohibit negative values considering it being a bug and have only a few > special cases around -1 that we convert. > > > Please check the matching patterns we use to accept config values in > DurationSpec, DataRateSpec and DataStorageSpec and let me know if you think > there was some special case missed or anything. > > > One more topic I want to mention is changes to public classes. There were > some name changes (not to JMX) and out of this work I’ve seen such changes > even in patch releases. I know we give a promise not to change only > interfaces but I still want to mention it here and to ensure we are aligned. > As I said, Config changes the types of our parameters. We use annotations and > the mentioned converters for backward compatibility of our yaml file. > > > New JMX methods are not added and the old ones still work by converting to > the base unit of the specific parameter. By base I mean the internally used > one. The agreement was that in the future virtual tables should handle the > new format when we add the update option for SettingsTable. > > > In order not to run into precision issues, we introduced the Smallest > possible unit for certain parameters of type Duration and DataStorage, that > can be changed if we decide to migrate any of those in time to the smallest > possible unit internally. I am also adding their Int equivalents now to > improve the former int parameters handling. > > We didn’t find any parameters with suffix _mb or _kb that internally didn’t > mean kibi- or mebi- but kilo- or mega-. So all default values are the same in > the new format. > > > For more details - please refer to the write up doc I shared before here - > https://cassandra.apache.org/doc/trunk/cassandra/new/configuration.html There > is important note and ticket around parameters overloading which turned to be > undocumented behavior in the project that people use. > > > Last but not least - I want to remind you that any new parameters are added > with the new types. Users will have to set them with their new format. So > whoever wants to use old yaml and wants to update default values of new > parameters will have to do this in the new format. > > > This email came rather long but I truly believe it is important and I want to > ask you if there were parameters you think they might have been affected, to > double check them and raise the flag if you are not sure or worried about > something done or not done. > > > [1] > https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/commitlog/GroupCommitLogService.java#L31 > > > > Ekaterina Dimitrova
