Skip to content

Commit

Permalink
exclude system tables and fix bug with default compressor when settin…
Browse files Browse the repository at this point in the history
…g enabled to true
  • Loading branch information
smiklosovic committed Aug 10, 2023
1 parent b5e3bf3 commit 30e13bd
Show file tree
Hide file tree
Showing 14 changed files with 134 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -526,14 +526,14 @@ public void validate(ClientState state)

Guardrails.tableProperties.guard(attrs.updatedProperties(), attrs::removeProperty, state);

validateDefaultTimeToLive(attrs.asNewTableParams());
validateDefaultTimeToLive(attrs.asNewTableParams(keyspaceName, tableName));
}

public KeyspaceMetadata apply(KeyspaceMetadata keyspace, TableMetadata table)
{
attrs.validate();
attrs.validate(keyspaceName, tableName);

TableParams params = attrs.asAlteredTableParams(table.params);
TableParams params = attrs.asAlteredTableParams(table.params, keyspaceName, tableName);

if (table.isCounter() && params.defaultTimeToLive > 0)
throw ire("Cannot set default_time_to_live on a table with counters");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ public Keyspaces apply(Keyspaces schema)
throw ire("Materialized view '%s.%s' doesn't exist", keyspaceName, viewName);
}

attrs.validate();
attrs.validate(keyspaceName, viewName);

// Guardrails on table properties
Guardrails.tableProperties.guard(attrs.updatedProperties(), attrs::removeProperty, state);

TableParams params = attrs.asAlteredTableParams(view.metadata.params);
TableParams params = attrs.asAlteredTableParams(view.metadata.params, keyspaceName, viewName);

if (params.gcGraceSeconds == 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public void validate(ClientState state)
if (useCompactStorage)
Guardrails.compactTablesEnabled.ensureEnabled(state);

validateDefaultTimeToLive(attrs.asNewTableParams());
validateDefaultTimeToLive(attrs.asNewTableParams(keyspaceName, tableName));

rawColumns.forEach((name, raw) -> raw.validate(state, name));
}
Expand Down Expand Up @@ -184,8 +184,8 @@ public String toString()

public TableMetadata.Builder builder(Types types)
{
attrs.validate();
TableParams params = attrs.asNewTableParams();
attrs.validate(keyspaceName, tableName);
TableParams params = attrs.asNewTableParams(keyspaceName, tableName);

// use a TreeMap to preserve ordering across JDK versions (see CASSANDRA-9492) - important for stable unit tests
Map<ColumnIdentifier, ColumnProperties> columns = new TreeMap<>(comparing(o -> o.bytes));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public Keyspaces apply(Keyspaces schema)
* Validate WITH params
*/

attrs.validate();
attrs.validate(keyspaceName, viewName);

if (attrs.hasOption(TableParams.Option.DEFAULT_TIME_TO_LIVE)
&& attrs.getInt(TableParams.Option.DEFAULT_TIME_TO_LIVE.toString(), 0) != 0)
Expand All @@ -324,7 +324,7 @@ public Keyspaces apply(Keyspaces schema)
if (attrs.hasProperty(TableAttributes.ID))
builder.id(attrs.getId());

builder.params(attrs.asNewTableParams())
builder.params(attrs.asNewTableParams(keyspaceName, viewName))
.kind(TableMetadata.Kind.VIEW);

partitionKeyColumns.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,22 @@ public final class TableAttributes extends PropertyDefinitions
obsoleteKeywords = ImmutableSet.of();
}

public void validate()
public void validate(String keyspace, String table)
{
validate(validKeywords, obsoleteKeywords);
build(TableParams.builder()).validate();
build(TableParams.builder(), keyspace, table).validate();
}

TableParams asNewTableParams()
TableParams asNewTableParams(String keyspace, String table)
{
return build(TableParams.builder());
return build(TableParams.builder(), keyspace, table);
}

TableParams asAlteredTableParams(TableParams previous)
TableParams asAlteredTableParams(TableParams previous, String keyspaceName, String tableName)
{
if (getId() != null)
throw new ConfigurationException("Cannot alter table id.");
return build(previous.unbuild());
return build(previous.unbuild(), keyspaceName, tableName);
}

public TableId getId() throws ConfigurationException
Expand All @@ -95,7 +95,7 @@ public static Set<String> allKeywords()
return Sets.union(validKeywords, obsoleteKeywords);
}

private TableParams build(TableParams.Builder builder)
private TableParams build(TableParams.Builder builder, String keyspace, String table)
{
if (hasOption(Option.ALLOW_AUTO_SNAPSHOT))
builder.allowAutoSnapshot(getBoolean(Option.ALLOW_AUTO_SNAPSHOT.toString(), true));
Expand Down Expand Up @@ -161,7 +161,7 @@ private TableParams build(TableParams.Builder builder)
if (hasOption(READ_REPAIR))
builder.readRepair(ReadRepairStrategy.fromString(getString(READ_REPAIR)));

return builder.build();
return builder.build(keyspace, table);
}

public boolean hasOption(Option option)
Expand Down
2 changes: 1 addition & 1 deletion src/java/org/apache/cassandra/db/ColumnFamilyStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -2964,7 +2964,7 @@ public void setCrcCheckChance(double crcCheckChance)
{
try
{
TableParams.builder().crcCheckChance(crcCheckChance).build().validate();
TableParams.builder().crcCheckChance(crcCheckChance).build(keyspace.getName(), name).validate();
for (ColumnFamilyStore cfs : concatWithIndexes())
{
cfs.crcCheckChance.set(crcCheckChance);
Expand Down
24 changes: 15 additions & 9 deletions src/java/org/apache/cassandra/schema/CompressionParams.java
Original file line number Diff line number Diff line change
Expand Up @@ -230,21 +230,27 @@ private static CompressionParams fromClassAndOptions(String sstableCompressionCl
max_compressed_length_in_bytes = CompressionParams.calcMaxCompressedLength(chunk_length_in_bytes, min_compress_ratio);
}

// try to set compressor type
CompressorType compressorType = StringUtils.isEmpty(sstableCompressionClass) ? DEFAULT_COMPRESSION_TYPE : null;
if (compressorType == null)
CompressorType compressorType = null;
try
{
try
compressorType = CompressorType.valueOf(sstableCompressionClass);
}
catch (Exception e)
{
// intentionally empty
}

Function<Map<String,String>, ICompressor> creator = compressorType != null ? compressorType.creator : (opt) -> {
if (sstableCompressionClass != null)
{
compressorType = CompressorType.valueOf(sstableCompressionClass);
return FBUtilities.newCompressor(parseCompressorClass(sstableCompressionClass), opt);
}
catch (IllegalArgumentException expected)
else
{
compressorType = CompressorType.forClass(sstableCompressionClass);
return FBUtilities.newCompressor(parseCompressorClass(defaultParams().klass().getName()), opt);
}
}
};

Function<Map<String,String>, ICompressor> creator = compressorType != null ? compressorType.creator : (opt) -> FBUtilities.newCompressor(parseCompressorClass(sstableCompressionClass), opt);
CompressionParams cp = new CompressionParams(enabled ? creator.apply(options) : null, chunk_length_in_bytes, max_compressed_length_in_bytes, min_compress_ratio, options);
if (enabled && compressorType != CompressorType.none)
{
Expand Down
11 changes: 10 additions & 1 deletion src/java/org/apache/cassandra/schema/SchemaKeyspace.java
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,16 @@ static TableParams createTableParamsFromRow(UntypedResultSet.Row row)
if (row.has("incremental_backups"))
builder.incrementalBackups(row.getBoolean("incremental_backups"));

return builder.build();
String tableName = null;

if (row.has("table_name"))
tableName = row.getString("table_name");
else if (row.has("view_name"))
tableName = row.getString("view_name");

assert tableName != null;

return builder.build(row.getString("keyspace_name"), tableName);
}

private static List<ColumnMetadata> fetchColumns(String keyspace, String table, Types types, UserFunctions functions)
Expand Down
8 changes: 6 additions & 2 deletions src/java/org/apache/cassandra/schema/TableMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ protected TableMetadata(Builder builder)

partitioner = builder.partitioner;
kind = builder.kind;
params = builder.params.build();
params = builder.params.build(keyspace, name);

indexName = kind == Kind.INDEX ? name.substring(name.indexOf('.') + 1) : null;

Expand Down Expand Up @@ -641,7 +641,11 @@ public TableMetadata updateIndexTableMetadata(TableParams baseTableParams)
// Row caching is never enabled; see CASSANDRA-5732
builder.caching(baseTableParams.caching.cacheKeys() ? CachingParams.CACHE_KEYS : CachingParams.CACHE_NOTHING);

return unbuild().params(builder.build()).build();
Builder unbuilt = unbuild();
String keyspace = unbuilt.keyspace;
String table = unbuilt.name;

return unbuilt.params(builder.build(keyspace, table)).build();
}

boolean referencesUserType(ByteBuffer name)
Expand Down
10 changes: 8 additions & 2 deletions src/java/org/apache/cassandra/schema/TableParams.java
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ public static final class Builder
private SpeculativeRetryPolicy additionalWritePolicy = PercentileSpeculativeRetryPolicy.NINETY_NINE_P;
private CachingParams caching = CachingParams.DEFAULT;
private CompactionParams compaction = CompactionParams.DEFAULT;
private CompressionParams compression = CompressionParams.defaultParams();
private CompressionParams compression = null;
private MemtableParams memtable = MemtableParams.DEFAULT;
private ImmutableMap<String, ByteBuffer> extensions = ImmutableMap.of();
private boolean cdc;
Expand All @@ -360,8 +360,14 @@ public Builder()
{
}

public TableParams build()
public TableParams build(String keyspace, String name)
{
if (compression == null)
if (SchemaConstants.getLocalAndReplicatedSystemKeyspaceNames().contains(keyspace))
compression = CompressionParams.DEFAULT;
else
compression = CompressionParams.defaultParams();

return new TableParams(this);
}

Expand Down
Loading

0 comments on commit 30e13bd

Please sign in to comment.