Skip to content

Commit

Permalink
Disable the deprecated keyspace/table thresholds and convert them to …
Browse files Browse the repository at this point in the history
…guardrails

patch by Daniel Jatnieks; reviewed by Andrés de la Peña and Brandon Williams for CASSANDRA-18617
  • Loading branch information
djatnieks authored and adelapena committed Jul 14, 2023
1 parent 903857b commit aac0706
Show file tree
Hide file tree
Showing 18 changed files with 222 additions and 137 deletions.
1 change: 1 addition & 0 deletions CHANGES.txt
@@ -1,4 +1,5 @@
5.0
* Disable the deprecated keyspace/table thresholds and convert them to guardrails (CASSANDRA-18617)
* Deprecate CloudstackSnitch and remove duplicate code in snitches (CASSANDRA-18438)
* Add support for vectors in UDFs (CASSANDRA-18613)
* Improve vector value validation errors (CASSANDRA-18652)
Expand Down
2 changes: 1 addition & 1 deletion NEWS.txt
Expand Up @@ -439,7 +439,7 @@ Deprecation
No change of functionality in the new one, only name change for clarity in regards to units and to follow naming
standartization.
- The properties `keyspace_count_warn_threshold` and `table_count_warn_threshold` in cassandra.yaml have been
deprecated in favour of the new `guardrails.keyspaces` and `guardrails.tables` properties and will be removed
deprecated in favour of the new `keyspaces_warn_threshold` and `tables_warn_threshold` properties and will be removed
in a subsequent major version. This also affects the setters and getters for those properties in the JMX MBean
`org.apache.cassandra.db:type=StorageService`, which are equally deprecated in favour of the analogous methods
in the JMX MBean `org.apache.cassandra.db:type=Guardrails`. See CASSANDRA-17195 for further details.
Expand Down
7 changes: 0 additions & 7 deletions conf/cassandra.yaml
Expand Up @@ -1685,13 +1685,6 @@ repaired_data_tracking_for_partition_reads_enabled: false
# mismatches are less actionable than confirmed ones.
report_unconfirmed_repaired_data_mismatches: false

# Having many tables and/or keyspaces negatively affects performance of many operations in the
# cluster. When the number of tables/keyspaces in the cluster exceeds the following thresholds
# a client warning will be sent back to the user when creating a table or keyspace.
# As of cassandra 4.1, these properties are deprecated in favor of keyspaces_warn_threshold and tables_warn_threshold
# table_count_warn_threshold: 150
# keyspace_count_warn_threshold: 40

# configure the read and write consistency levels for modifications to auth tables
# auth_read_consistency_level: LOCAL_QUORUM
# auth_write_consistency_level: EACH_QUORUM
Expand Down
4 changes: 4 additions & 0 deletions src/java/org/apache/cassandra/auth/AuthKeyspace.java
Expand Up @@ -17,8 +17,11 @@
*/
package org.apache.cassandra.auth;

import java.util.Set;
import java.util.concurrent.TimeUnit;

import com.google.common.collect.ImmutableSet;

import org.apache.cassandra.config.CassandraRelevantProperties;
import org.apache.cassandra.cql3.statements.schema.CreateTableStatement;
import org.apache.cassandra.config.DatabaseDescriptor;
Expand Down Expand Up @@ -55,6 +58,7 @@ private AuthKeyspace()
public static final String ROLE_PERMISSIONS = "role_permissions";
public static final String RESOURCE_ROLE_INDEX = "resource_role_permissons_index";
public static final String NETWORK_PERMISSIONS = "network_permissions";
public static final Set<String> TABLE_NAMES = ImmutableSet.of(ROLES, ROLE_MEMBERS, ROLE_PERMISSIONS, RESOURCE_ROLE_INDEX, NETWORK_PERMISSIONS);

public static final long SUPERUSER_SETUP_DELAY = SUPERUSER_SETUP_DELAY_MS.getLong();

Expand Down
7 changes: 2 additions & 5 deletions src/java/org/apache/cassandra/config/Config.java
Expand Up @@ -826,18 +826,15 @@ public static void setClientMode(boolean clientMode)
isClientMode = clientMode;
}

@Deprecated // this warning threshold will be replaced by an equivalent guardrail
public volatile int table_count_warn_threshold = 150;
@Deprecated // this warning threshold will be replaced by an equivalent guardrail
public volatile int keyspace_count_warn_threshold = 40;

public volatile int consecutive_message_errors_threshold = 1;

public volatile SubnetGroups client_error_reporting_exclusions = new SubnetGroups();
public volatile SubnetGroups internode_error_reporting_exclusions = new SubnetGroups();

@Replaces(oldName = "keyspace_count_warn_threshold", converter = Converters.KEYSPACE_COUNT_THRESHOLD_TO_GUARDRAIL, deprecated = true)
public volatile int keyspaces_warn_threshold = -1;
public volatile int keyspaces_fail_threshold = -1;
@Replaces(oldName = "table_count_warn_threshold", converter = Converters.TABLE_COUNT_THRESHOLD_TO_GUARDRAIL, deprecated = true)
public volatile int tables_warn_threshold = -1;
public volatile int tables_fail_threshold = -1;
public volatile int columns_per_table_warn_threshold = -1;
Expand Down
10 changes: 9 additions & 1 deletion src/java/org/apache/cassandra/config/Converters.java
Expand Up @@ -21,6 +21,8 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Function;

import org.apache.cassandra.schema.SchemaConstants;

import static org.apache.cassandra.config.DataRateSpec.DataRateUnit.MEBIBYTES_PER_SECOND;

/**
Expand Down Expand Up @@ -117,7 +119,13 @@ public enum Converters
*/
MEGABITS_TO_BYTES_PER_SECOND_DATA_RATE(Integer.class, DataRateSpec.LongBytesPerSecondBound.class,
i -> DataRateSpec.LongBytesPerSecondBound.megabitsPerSecondInBytesPerSecond(i),
o -> o == null ? null : o.toMegabitsPerSecondAsInt());
o -> o == null ? null : o.toMegabitsPerSecondAsInt()),
KEYSPACE_COUNT_THRESHOLD_TO_GUARDRAIL(int.class, int.class,
i -> i - SchemaConstants.getLocalAndReplicatedSystemKeyspaceNames().size(),
o -> o == null ? null : o + SchemaConstants.getLocalAndReplicatedSystemKeyspaceNames().size()),
TABLE_COUNT_THRESHOLD_TO_GUARDRAIL(int.class, int.class,
i -> i - SchemaConstants.getLocalAndReplicatedSystemTableNames().size(),
o -> o == null ? null : o + SchemaConstants.getLocalAndReplicatedSystemTableNames().size());
private final Class<?> oldType;
private final Class<?> newType;
private final Function<Object, Object> convert;
Expand Down
24 changes: 0 additions & 24 deletions src/java/org/apache/cassandra/config/DatabaseDescriptor.java
Expand Up @@ -4195,30 +4195,6 @@ public static void setAutoOptimisePreviewRepairStreams(boolean enabled)
conf.auto_optimise_preview_repair_streams = enabled;
}

@Deprecated
public static int tableCountWarnThreshold()
{
return conf.table_count_warn_threshold;
}

@Deprecated // this warning threshold will be replaced by an equivalent guardrail
public static void setTableCountWarnThreshold(int value)
{
conf.table_count_warn_threshold = value;
}

@Deprecated // this warning threshold will be replaced by an equivalent guardrail
public static int keyspaceCountWarnThreshold()
{
return conf.keyspace_count_warn_threshold;
}

@Deprecated // this warning threshold will be replaced by an equivalent guardrail
public static void setKeyspaceCountWarnThreshold(int value)
{
conf.keyspace_count_warn_threshold = value;
}

@Deprecated // this warning threshold will be replaced by an equivalent guardrail
public static ConsistencyLevel getAuthWriteConsistencyLevel()
{
Expand Down
Expand Up @@ -31,7 +31,6 @@
import org.apache.cassandra.auth.FunctionResource;
import org.apache.cassandra.auth.IResource;
import org.apache.cassandra.auth.Permission;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.cql3.CQLStatement;
import org.apache.cassandra.db.guardrails.Guardrails;
import org.apache.cassandra.exceptions.AlreadyExistsException;
Expand Down Expand Up @@ -124,22 +123,6 @@ public void validate(ClientState state)
Guardrails.keyspaces.guard(Schema.instance.getUserKeyspaces().size() + 1, keyspaceName, false, state);
}

@Override
Set<String> clientWarnings(KeyspacesDiff diff)
{
// this threshold is deprecated, it will be replaced by the guardrail used in #validate(ClientState)
int keyspaceCount = Schema.instance.getKeyspaces().size();
if (keyspaceCount > DatabaseDescriptor.keyspaceCountWarnThreshold())
{
String msg = String.format("Cluster already contains %d keyspaces. Having a large number of keyspaces will significantly slow down schema dependent cluster operations.",
keyspaceCount);
logger.warn(msg);
clientWarnings.add(msg);
}

return clientWarnings;
}

public static final class Raw extends CQLStatement.Raw
{
public final String keyspaceName;
Expand Down
Expand Up @@ -34,7 +34,6 @@
import org.apache.cassandra.auth.DataResource;
import org.apache.cassandra.auth.IResource;
import org.apache.cassandra.auth.Permission;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.cql3.*;
import org.apache.cassandra.cql3.functions.masking.ColumnMask;
import org.apache.cassandra.db.Keyspace;
Expand Down Expand Up @@ -426,22 +425,6 @@ else if (!builder.hasRegularColumns())
}
}

@Override
public Set<String> clientWarnings(KeyspacesDiff diff)
{
// this threshold is deprecated, it will be replaced by the guardrail used in #validate(ClientState)
int tableCount = Schema.instance.getNumberOfTables();
if (tableCount > DatabaseDescriptor.tableCountWarnThreshold())
{
String msg = String.format("Cluster already contains %d tables in %d keyspaces. Having a large number of tables will significantly slow down schema dependent cluster operations.",
tableCount,
Schema.instance.getKeyspaces().size());
logger.warn(msg);
return ImmutableSet.of(msg);
}
return ImmutableSet.of();
}

private static class DefaultNames
{
private static final String DEFAULT_CLUSTERING_NAME = "column";
Expand Down
6 changes: 6 additions & 0 deletions src/java/org/apache/cassandra/db/SystemKeyspace.java
Expand Up @@ -191,6 +191,12 @@ private SystemKeyspace()
BUILT_VIEWS, PREPARED_STATEMENTS, REPAIRS, TOP_PARTITIONS, LEGACY_PEERS, LEGACY_PEER_EVENTS,
LEGACY_TRANSFERRED_RANGES, LEGACY_AVAILABLE_RANGES, LEGACY_SIZE_ESTIMATES, LEGACY_SSTABLE_ACTIVITY);

public static final Set<String> TABLE_NAMES = ImmutableSet.of(
BATCHES, PAXOS, PAXOS_REPAIR_HISTORY, BUILT_INDEXES, LOCAL, PEERS_V2, PEER_EVENTS_V2,
COMPACTION_HISTORY, SSTABLE_ACTIVITY_V2, TABLE_ESTIMATES, AVAILABLE_RANGES_V2, TRANSFERRED_RANGES_V2, VIEW_BUILDS_IN_PROGRESS,
BUILT_VIEWS, PREPARED_STATEMENTS, REPAIRS, TOP_PARTITIONS, LEGACY_PEERS, LEGACY_PEER_EVENTS,
LEGACY_TRANSFERRED_RANGES, LEGACY_AVAILABLE_RANGES, LEGACY_SIZE_ESTIMATES, LEGACY_SSTABLE_ACTIVITY);

public static final TableMetadata Batches =
parse(BATCHES,
"batches awaiting replay",
Expand Down
27 changes: 27 additions & 0 deletions src/java/org/apache/cassandra/schema/SchemaConstants.java
Expand Up @@ -27,7 +27,10 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;

import org.apache.cassandra.auth.AuthKeyspace;
import org.apache.cassandra.db.Digest;
import org.apache.cassandra.db.SystemKeyspace;
import org.apache.cassandra.tracing.TraceKeyspace;

/**
* When adding new String keyspace names here, double check if it needs to be added to PartitionDenylist.canDenylistKeyspace
Expand Down Expand Up @@ -126,4 +129,28 @@ public static Set<String> getSystemKeyspaces()
{
return Sets.union(Sets.union(LOCAL_SYSTEM_KEYSPACE_NAMES, REPLICATED_SYSTEM_KEYSPACE_NAMES), VIRTUAL_SYSTEM_KEYSPACE_NAMES);
}

/**
* Returns the set of local and replicated system keyspace names
* @return all local and replicated system keyspace names
*/
public static Set<String> getLocalAndReplicatedSystemKeyspaceNames()
{
return Sets.union(LOCAL_SYSTEM_KEYSPACE_NAMES, REPLICATED_SYSTEM_KEYSPACE_NAMES);
}

/**
* Returns the set of all local and replicated system table names
* @return all local and replicated system table names
*/
public static Set<String> getLocalAndReplicatedSystemTableNames()
{
return ImmutableSet.<String>builder()
.addAll(SystemKeyspace.TABLE_NAMES)
.addAll(SchemaKeyspaceTables.ALL)
.addAll(TraceKeyspace.TABLE_NAMES)
.addAll(AuthKeyspace.TABLE_NAMES)
.addAll(SystemDistributedKeyspace.TABLE_NAMES)
.build();
}
}
Expand Up @@ -31,6 +31,7 @@
import java.util.concurrent.TimeUnit;

import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Sets;
Expand Down Expand Up @@ -93,6 +94,8 @@ private SystemDistributedKeyspace()

public static final String PARTITION_DENYLIST_TABLE = "partition_denylist";

public static final Set<String> TABLE_NAMES = ImmutableSet.of(REPAIR_HISTORY, PARENT_REPAIR_HISTORY, VIEW_BUILD_STATUS, PARTITION_DENYLIST_TABLE);

private static final TableMetadata RepairHistory =
parse(REPAIR_HISTORY,
"Repair history",
Expand Down
11 changes: 7 additions & 4 deletions src/java/org/apache/cassandra/service/StorageService.java
Expand Up @@ -107,6 +107,7 @@
import org.apache.cassandra.config.CassandraRelevantProperties;
import org.apache.cassandra.config.Config;
import org.apache.cassandra.config.Config.PaxosStatePurging;
import org.apache.cassandra.config.Converters;
import org.apache.cassandra.config.DataStorageSpec;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.config.DurationSpec;
Expand Down Expand Up @@ -6970,7 +6971,7 @@ public void setAutoOptimisePreviewRepairStreams(boolean enabled)
@Deprecated
public int getTableCountWarnThreshold()
{
return DatabaseDescriptor.tableCountWarnThreshold();
return (int) Converters.TABLE_COUNT_THRESHOLD_TO_GUARDRAIL.unconvert(Guardrails.instance.getTablesWarnThreshold());
}

@Deprecated
Expand All @@ -6979,13 +6980,14 @@ public void setTableCountWarnThreshold(int value)
if (value < 0)
throw new IllegalStateException("Table count warn threshold should be positive, not "+value);
logger.info("Changing table count warn threshold from {} to {}", getTableCountWarnThreshold(), value);
DatabaseDescriptor.setTableCountWarnThreshold(value);
Guardrails.instance.setTablesThreshold((int) Converters.TABLE_COUNT_THRESHOLD_TO_GUARDRAIL.convert(value),
Guardrails.instance.getTablesFailThreshold());
}

@Deprecated
public int getKeyspaceCountWarnThreshold()
{
return DatabaseDescriptor.keyspaceCountWarnThreshold();
return (int) Converters.KEYSPACE_COUNT_THRESHOLD_TO_GUARDRAIL.unconvert(Guardrails.instance.getKeyspacesWarnThreshold());
}

@Deprecated
Expand All @@ -6994,7 +6996,8 @@ public void setKeyspaceCountWarnThreshold(int value)
if (value < 0)
throw new IllegalStateException("Keyspace count warn threshold should be positive, not "+value);
logger.info("Changing keyspace count warn threshold from {} to {}", getKeyspaceCountWarnThreshold(), value);
DatabaseDescriptor.setKeyspaceCountWarnThreshold(value);
Guardrails.instance.setKeyspacesThreshold((int) Converters.KEYSPACE_COUNT_THRESHOLD_TO_GUARDRAIL.convert(value),
Guardrails.instance.getKeyspacesFailThreshold());
}

@Override
Expand Down
3 changes: 3 additions & 0 deletions src/java/org/apache/cassandra/tracing/TraceKeyspace.java
Expand Up @@ -21,6 +21,8 @@
import java.nio.ByteBuffer;
import java.util.*;

import com.google.common.collect.ImmutableSet;

import org.apache.cassandra.config.CassandraRelevantProperties;
import org.apache.cassandra.cql3.statements.schema.CreateTableStatement;
import org.apache.cassandra.config.DatabaseDescriptor;
Expand Down Expand Up @@ -66,6 +68,7 @@ private TraceKeyspace()

public static final String SESSIONS = "sessions";
public static final String EVENTS = "events";
public static final Set<String> TABLE_NAMES = ImmutableSet.of(SESSIONS, EVENTS);

private static final TableMetadata Sessions =
parse(SESSIONS,
Expand Down

0 comments on commit aac0706

Please sign in to comment.