apache
17737 4.1 sstable preemptive open interval in mb
About the VT, the conversion happens in the last method in the converter which was not handling the special case, I will fix it in a moment.
The reverseConverter in Converter is what handles this, it should be in this case o -> o == null ? -1 : o.toMebibytes()),
Ha...about the VT, it seems that someone added null check in the unconvert() method and broke the special cases we have in the converter... I will see to fix it and add more tests to prevent that from happening in the future
@jonmeredith I just pushed review commits to address your feedback and follow up discussion plus improved the testing. The only current issue I see is with the fromMap in the configuration loader which does not accept currently null values for properties that are not null by default in the Config class. I will try to figure it out tomorrow but at least we have a workaround and I wanted to show you what I came up with so far. Currently that method is also only used by the in-jvm test framework.
The Converter was broken when we introduced null post CASSANDRA-15234, I will add to the new tests for the rest of the properties and converters in a separate commit tomorrow. I didn't want to overwhelm this one. Only fixed the issue with index_summary_resize_interval=null which was the same as the one with sstable_preemptive_open_interval=null on startup
@adelapena I addressed your feedback. The new tests pass locally, but I started full CI #1823 https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra?branch=17737-4.1-sstable_preemptive_open_interval_in_mb
CI shows some issue with @Nullable, I think I need to change the scope in build.xml for the annotation, I will work it now. I will let you know when I have the fix pushed
com.google.code.findbugs needs to have default compile and not provided scope for this patch to work. Looking into its license https://opensource.org/licenses/BSD-3-Clause and reading here - https://www.apache.org/legal/resolved.html I think this is fine. Do we need to bring it to the mailing list? I think only introducing brand new dependencies? WDYT? CI - https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra?branch=17737-4.1-sstable_preemptive_open_interval_in_mb #1824 still running
The last changes look good to me. It seems that CI hasn't been started. Also we'd need to apply the changes to the patch for trunk.
