Remove deprecated compaction_tombstone_warning_threshold and compaction_large_partition_warning_threshold from yaml

patch by Andrés de la Peña; reviewed by Brandon Williams for CASSANDRA-18626
This commit is contained in:
Andrés de la Peña 2023-06-27 16:58:16 +01:00
parent e4c55bf3c1
commit c579faa488
14 changed files with 46 additions and 62 deletions

View File

@ -1,4 +1,5 @@
5.0
* Remove deprecated compaction_tombstone_warning_threshold and compaction_large_partition_warning_threshold from yaml (CASSANDRA-18626)
* Enhance nodetool compactionstats with additional metrics (CASSANDRA-18305)
* Added support for type VECTOR<type, dimension> (CASSANDRA-18504)
* Expose bootstrap and decommission state to nodetool info (CASSANDRA-18555)

View File

@ -218,9 +218,11 @@ Deprecation
- The config property `compaction_large_partition_warning_threshold` has been deprecated in favour of the new
guardrail for partition size. That guardrail is based on the properties `partition_size_warn_threshold` and
`partition_size_fail_threshold`. The warn threshold has a very similar behaviour to the old config property.
The old property is still supported for backward compatibility, but now it is disabled by default.
- The config property `compaction_tombstone_warning_threshold` has been deprecated in favour of the new guardrail
for partition tombstones. That guardrail is based on the properties `partition_tombstones_warn_threshold` and
`partition_tombstones_fail_threshold`. The warn threshold has a very similar behaviour to the old config property.
The old property is still supported for backward compatibility, but now it is disabled by default.
4.1
===

View File

@ -1560,14 +1560,6 @@ batch_size_fail_threshold: 50KiB
# Log WARN on any batches not of type LOGGED than span across more partitions than this limit
unlogged_batch_across_partitions_warn_threshold: 10
# Log a warning when compacting partitions larger than this value.
# As of Cassandra 5.0, this property is deprecated in favour of partition_size_warn_threshold.
compaction_large_partition_warning_threshold: 100MiB
# Log a warning when writing more tombstones than this value to a partition.
# As of Cassandra 5.0, this property is deprecated in favour of partition_tombstones_warn_threshold.
compaction_tombstone_warning_threshold: 100000
# GC Pauses greater than 200 ms will be logged at INFO level
# This threshold can be adjusted to minimize logging if necessary
# Min unit: ms

View File

@ -328,13 +328,8 @@ public class Config
public volatile Integer concurrent_compactors;
@Replaces(oldName = "compaction_throughput_mb_per_sec", converter = Converters.MEBIBYTES_PER_SECOND_DATA_RATE, deprecated = true)
public volatile DataRateSpec.LongBytesPerSecondBound compaction_throughput = new DataRateSpec.LongBytesPerSecondBound("64MiB/s");
@Deprecated
@Replaces(oldName = "compaction_large_partition_warning_threshold_mb", converter = Converters.MEBIBYTES_DATA_STORAGE_INT, deprecated = true)
public volatile DataStorageSpec.IntMebibytesBound compaction_large_partition_warning_threshold = new DataStorageSpec.IntMebibytesBound("100MiB");
@Replaces(oldName = "min_free_space_per_drive_in_mb", converter = Converters.MEBIBYTES_DATA_STORAGE_INT, deprecated = true)
public DataStorageSpec.IntMebibytesBound min_free_space_per_drive = new DataStorageSpec.IntMebibytesBound("50MiB");
@Deprecated
public volatile Integer compaction_tombstone_warning_threshold = 100000;
// fraction of free disk space available for compaction after min free space is subtracted
public volatile Double max_space_usable_for_compactions_in_percentage = .95;
@ -875,8 +870,11 @@ public class Config
public volatile boolean read_before_write_list_operations_enabled = true;
public volatile boolean allow_filtering_enabled = true;
public volatile boolean simplestrategy_enabled = true;
@Replaces(oldName = "compaction_large_partition_warning_threshold_mb", converter = Converters.LONG_BYTES_DATASTORAGE_MEBIBYTES_INT, deprecated = true)
@Replaces(oldName = "compaction_large_partition_warning_threshold", converter = Converters.LONG_BYTES_DATASTORAGE_MEBIBYTES_DATASTORAGE, deprecated = true)
public volatile DataStorageSpec.LongBytesBound partition_size_warn_threshold = null;
public volatile DataStorageSpec.LongBytesBound partition_size_fail_threshold = null;
@Replaces(oldName = "compaction_tombstone_warning_threshold", converter = Converters.IDENTITY, deprecated = true)
public volatile long partition_tombstones_warn_threshold = -1;
public volatile long partition_tombstones_fail_threshold = -1;
public volatile DataStorageSpec.LongBytesBound column_value_size_warn_threshold = null;

View File

@ -94,6 +94,12 @@ public enum Converters
BYTES_DATASTORAGE(Integer.class, DataStorageSpec.IntBytesBound.class,
DataStorageSpec.IntBytesBound::new,
o -> o == null ? null : o.toBytes()),
LONG_BYTES_DATASTORAGE_MEBIBYTES_INT(Integer.class, DataStorageSpec.LongBytesBound.class,
o -> o == null ? null : new DataStorageSpec.LongBytesBound(o, DataStorageSpec.DataStorageUnit.MEBIBYTES),
o -> o == null ? null : o.toMebibytesInt()),
LONG_BYTES_DATASTORAGE_MEBIBYTES_DATASTORAGE(DataStorageSpec.IntMebibytesBound.class, DataStorageSpec.LongBytesBound.class,
o -> o == null ? null : new DataStorageSpec.LongBytesBound(o.toBytesInLong()),
o -> o == null ? null : new DataStorageSpec.IntMebibytesBound(o.toMebibytesInt())),
/**
* This converter is used to support backward compatibility for parameters where in the past negative number was used as a value
* Example: native_transport_max_concurrent_requests_in_bytes_per_ip = -1 and native_transport_max_request_data_in_flight_per_ip = null

View File

@ -207,6 +207,14 @@ public abstract class DataStorageSpec
{
return unit().toBytes(quantity());
}
/**
* @return the amount of data storage in mebibytes
*/
public int toMebibytesInt()
{
return Ints.saturatedCast(unit().toMebibytes(quantity()));
}
}
/**

View File

@ -2175,24 +2175,6 @@ public class DatabaseDescriptor
conf.compaction_throughput = new DataRateSpec.LongBytesPerSecondBound(value, MEBIBYTES_PER_SECOND);
}
@Deprecated
public static long getCompactionLargePartitionWarningThreshold()
{
return conf.compaction_large_partition_warning_threshold.toBytesInLong();
}
@Deprecated
public static int getCompactionTombstoneWarningThreshold()
{
return conf.compaction_tombstone_warning_threshold;
}
@Deprecated
public static void setCompactionTombstoneWarningThreshold(int count)
{
conf.compaction_tombstone_warning_threshold = count;
}
public static int getConcurrentValidations()
{
return conf.concurrent_validations;

View File

@ -27,7 +27,6 @@ import com.google.common.collect.ImmutableSet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.db.DecoratedKey;
import org.apache.cassandra.db.DeletionPurger;
import org.apache.cassandra.db.DeletionTime;
@ -216,8 +215,6 @@ public abstract class SortedTableWriter<P extends SortedTablePartitionWriter> ex
long rowSize = endPosition - partitionWriter.getInitialPosition();
guardPartitionThreshold(Guardrails.partitionSize, key, rowSize);
guardPartitionThreshold(Guardrails.partitionTombstones, key, metadataCollector.totalTombstones);
maybeLogLargePartitionWarning(key, rowSize);
maybeLogManyTombstonesWarning(key, metadataCollector.totalTombstones);
metadataCollector.addPartitionSizeInBytes(rowSize);
metadataCollector.addKey(key.getKey());
metadataCollector.addCellPerPartitionCount();
@ -340,26 +337,6 @@ public abstract class SortedTableWriter<P extends SortedTablePartitionWriter> ex
}
}
@Deprecated
private void maybeLogLargePartitionWarning(DecoratedKey key, long rowSize)
{
if (rowSize > DatabaseDescriptor.getCompactionLargePartitionWarningThreshold())
{
String keyString = metadata().partitionKeyType.getString(key.getKey());
logger.warn("Writing large partition {}/{}:{} ({}) to sstable {}", metadata.keyspace, metadata.name, keyString, FBUtilities.prettyPrintMemory(rowSize), getFilename());
}
}
@Deprecated
private void maybeLogManyTombstonesWarning(DecoratedKey key, int tombstoneCount)
{
if (tombstoneCount > DatabaseDescriptor.getCompactionTombstoneWarningThreshold())
{
String keyString = metadata().partitionKeyType.getString(key.getKey());
logger.warn("Writing {} tombstones to {}/{}:{} in sstable {}", tombstoneCount, metadata.keyspace, metadata.name, keyString, getFilename());
}
}
private void guardCollectionSize(DecoratedKey partitionKey, Row row)
{
if (!Guardrails.collectionSize.enabled() && !Guardrails.itemsPerCollection.enabled())

View File

@ -121,6 +121,7 @@ import org.apache.cassandra.db.SystemKeyspace;
import org.apache.cassandra.db.commitlog.CommitLog;
import org.apache.cassandra.db.compaction.CompactionManager;
import org.apache.cassandra.db.compaction.OperationType;
import org.apache.cassandra.db.guardrails.Guardrails;
import org.apache.cassandra.db.lifecycle.LifecycleTransaction;
import org.apache.cassandra.db.virtual.VirtualKeyspaceRegistry;
import org.apache.cassandra.dht.BootStrapper;
@ -7002,13 +7003,13 @@ public class StorageService extends NotificationBroadcasterSupport implements IE
if (count < 0)
throw new IllegalStateException("compaction tombstone warning threshold needs to be >= 0, not "+count);
logger.info("Setting compaction_tombstone_warning_threshold to {}", count);
DatabaseDescriptor.setCompactionTombstoneWarningThreshold(count);
Guardrails.instance.setPartitionTombstonesThreshold(count, Guardrails.instance.getPartitionTombstonesFailThreshold());
}
@Override
public int getCompactionTombstoneWarningThreshold()
{
return DatabaseDescriptor.getCompactionTombstoneWarningThreshold();
return Math.toIntExact(Guardrails.instance.getPartitionTombstonesWarnThreshold());
}
public void addSnapshot(TableSnapshot snapshot) {

View File

@ -123,7 +123,8 @@ public class TombstoneWarningTest extends TestBaseImpl
{
long mark = cluster.get(1).logs().mark();
cluster.get(1).flush(KEYSPACE);
String pattern = ".*Writing (?<tscount>\\d+) tombstones to distributed_test_keyspace/tbl:(?<key>\\d+).*";
String pattern = ".*Guardrail partition_tombstones violated: " +
"Partition distributed_test_keyspace\\.tbl:(?<key>\\d+).* has (?<tscount>\\d+) tombstones.*";
LogResult<List<String>> res = cluster.get(1).logs().grep(mark, pattern);
assertEquals(expectedCount, res.getResult().size());
Pattern p = Pattern.compile(pattern);

View File

@ -53,7 +53,6 @@ public class GuardrailPartitionSizeTest extends GuardrailTester
cluster = init(Cluster.build(NUM_NODES)
.withConfig(c -> c.set("partition_size_warn_threshold", WARN_THRESHOLD + "B")
.set("partition_size_fail_threshold", FAIL_THRESHOLD + "B")
.set("compaction_large_partition_warning_threshold", "999GiB")
.set("memtable_heap_space", "512MiB")) // avoids flushes
.start());
cluster.disableAutoCompaction(KEYSPACE);

View File

@ -330,9 +330,6 @@ public class DatabaseDescriptorTest
long maxIntMebibytesAsBytes = (long) maxInt * 1024 * 1024;
long maxIntKibibytesAsBytes = (long) maxInt * 1024;
conf.compaction_large_partition_warning_threshold = new DataStorageSpec.IntMebibytesBound(maxInt);
Assert.assertEquals(maxIntMebibytesAsBytes, DatabaseDescriptor.getCompactionLargePartitionWarningThreshold());
conf.min_free_space_per_drive = new DataStorageSpec.IntMebibytesBound(maxInt);
Assert.assertEquals(maxIntMebibytesAsBytes, DatabaseDescriptor.getMinFreeSpacePerDriveInBytes());

View File

@ -92,7 +92,6 @@ public class ParseAndConvertUnitsTest
assertEquals(new DataStorageSpec.IntKibibytesBound(2), config.column_index_cache_size);
assertEquals(new DataStorageSpec.IntKibibytesBound(5), config.batch_size_warn_threshold);
assertEquals(new DataStorageSpec.IntKibibytesBound(50), config.batch_size_fail_threshold);
assertEquals(new DataStorageSpec.IntMebibytesBound(100), config.compaction_large_partition_warning_threshold);
assertNull(config.commitlog_total_space);
assertEquals(new DataStorageSpec.IntMebibytesBound(5), config.commitlog_segment_size);
assertNull(config.max_mutation_size); //not set explicitly in the default yaml, check the config; not set there too

View File

@ -355,6 +355,27 @@ public class YamlConfigurationLoaderTest
// NEGATIVE_MEBIBYTES_DATA_STORAGE_INT
assertThat(from("sstable_preemptive_open_interval_in_mb", "1").sstable_preemptive_open_interval.toMebibytes()).isEqualTo(1);
assertThat(from("sstable_preemptive_open_interval_in_mb", -2).sstable_preemptive_open_interval).isNull();
// LONG_BYTES_DATASTORAGE_MEBIBYTES_INT
assertThat(from("compaction_large_partition_warning_threshold_mb", "42").partition_size_warn_threshold.toMebibytesInt()).isEqualTo(42);
assertThatThrownBy(() -> from("compaction_large_partition_warning_threshold_mb", -1).partition_size_warn_threshold.toMebibytesInt())
.hasRootCauseInstanceOf(IllegalArgumentException.class)
.hasRootCauseMessage("Invalid data storage: value must be non-negative");
// LONG_BYTES_DATASTORAGE_MEBIBYTES_DATASTORAGE
assertThat(from("compaction_large_partition_warning_threshold", "42MiB").partition_size_warn_threshold.toMebibytesInt()).isEqualTo(42);
assertThat(from("compaction_large_partition_warning_threshold", "42GiB").partition_size_warn_threshold.toMebibytesInt()).isEqualTo(42 * 1024);
assertThatThrownBy(() -> from("compaction_large_partition_warning_threshold", "42B").partition_size_warn_threshold.toBytes())
.hasRootCauseInstanceOf(IllegalArgumentException.class)
.hasRootCauseMessage("Invalid data storage: 42B Accepted units:[MEBIBYTES, GIBIBYTES]");
assertThatThrownBy(() -> from("compaction_large_partition_warning_threshold", -1).partition_size_warn_threshold.toMebibytesInt())
.hasRootCauseInstanceOf(IllegalArgumentException.class)
.hasRootCauseMessage("Invalid data storage: -1 Accepted units:[MEBIBYTES, GIBIBYTES] where case matters and only non-negative values are accepted");
// IDENTITY
assertThat(from("compaction_tombstone_warning_threshold", "42").partition_tombstones_warn_threshold).isEqualTo(42);
assertThat(from("compaction_tombstone_warning_threshold", "-1").partition_tombstones_warn_threshold).isEqualTo(-1);
assertThat(from("compaction_tombstone_warning_threshold", "0").partition_tombstones_warn_threshold).isEqualTo(0);
}
private static Config from(Object... values)