adelapena commented on code in PR #1758:
URL: https://github.com/apache/cassandra/pull/1758#discussion_r932174326
##########
.build/build-rat.xml:
##########
@@ -58,6 +58,8 @@
<exclude NAME="**/doc/antora.yml"/>
<exclude name="**/test/conf/cassandra.yaml"/>
<exclude name="**/test/conf/cassandra-old.yaml"/>
+ <exclude
name="**/cassandra-converters-special-cases-old-names.yaml"/>
Review Comment:
I think this should be `<exclude
name="**/test/conf/cassandra-converters-special-cases-old-names.yaml"/>`
##########
test/conf/cassandra-converters-special-cases-old-names.yaml:
##########
@@ -0,0 +1,3 @@
+sstable_preemptive_open_interval_in_mb: -1
Review Comment:
Maybe we could add a brief comment at the beginning of this file indicating
what it's used for.
##########
src/java/org/apache/cassandra/config/Config.java:
##########
@@ -423,7 +425,8 @@ public MemtableOptions()
@Replaces(oldName = "trickle_fsync_interval_in_kb", converter =
Converters.KIBIBYTES_DATASTORAGE, deprecated = true)
public DataStorageSpec.IntKibibytesBound trickle_fsync_interval = new
DataStorageSpec.IntKibibytesBound("10240KiB");
- @Replaces(oldName = "sstable_preemptive_open_interval_in_mb", converter =
Converters.MEBIBYTES_DATA_STORAGE_INT, deprecated = true)
+ @Nullable
+ @Replaces(oldName = "sstable_preemptive_open_interval_in_mb", converter =
Converters.NEGATIVE_DATA_STORAGE_INT, deprecated = true)
Review Comment:
Should't this keep including `MEBIBYTES` in the name, as in
`NEGATIVE_MEBIBYTES_DATA_STORAGE_INT` or `MEBIBYTES_CUSTOM_DATA_STORAGE_INT`?
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -3074,12 +3074,17 @@ public static void
setMigrateKeycacheOnCompaction(boolean migrateCacheEntry)
public static int getSSTablePreemptiveOpenIntervalInMiB()
{
+ if (conf.sstable_preemptive_open_interval == null)
Review Comment:
I'd add some JavaDoc to the getter mentioning that it might return negative.
##########
test/conf/cassandra-converters-special-cases.yaml:
##########
@@ -0,0 +1,3 @@
+sstable_preemptive_open_interval:
Review Comment:
Maybe we could add a brief comment at the beginning of this file indicating
what it's used for.
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -3074,12 +3074,17 @@ public static void
setMigrateKeycacheOnCompaction(boolean migrateCacheEntry)
public static int getSSTablePreemptiveOpenIntervalInMiB()
{
+ if (conf.sstable_preemptive_open_interval == null)
+ return -1;
return conf.sstable_preemptive_open_interval.toMebibytes();
}
public static void setSSTablePreemptiveOpenIntervalInMiB(int mib)
{
- conf.sstable_preemptive_open_interval = new
DataStorageSpec.IntMebibytesBound(mib);
+ if (mib < 0)
Review Comment:
I'd add some JavaDoc to this setter mentioning that it accepts negative
values.
##########
NEWS.txt:
##########
@@ -165,6 +165,9 @@ New features
Upgrading
---------
+ - `sstable_preemptive_open_interval_in_mb` being negatve for disabled is
equivalent to `sstable_preemptive_open_interval`
Review Comment:
```suggestion
- `sstable_preemptive_open_interval_in_mb` being negative for disabled
is equivalent to `sstable_preemptive_open_interval`
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
