dcapwell commented on code in PR #2295:
URL: https://github.com/apache/cassandra/pull/2295#discussion_r1176780446
##########
src/java/org/apache/cassandra/config/Config.java:
##########
@@ -359,9 +352,14 @@ public MemtableOptions()
public String[] data_file_directories = new String[0];
- public List<ParameterizedClass> sstable_formats = ImmutableList.of(new
ParameterizedClass(BigFormat.class.getName(),//
"org.apache.cassandra.io.sstable.format.big.BigFormat",
-
ImmutableMap.of(SSTABLE_FORMAT_ID, "0",
-
SSTABLE_FORMAT_NAME, "big")));
+ public static class SSTableFormats
+ {
+ public String selected_format;
+ public String selected_version;
+ public Map<String, Map<String, String>> options = new HashMap<>();
+ }
+
+ public SSTableFormats sstable_formats = new SSTableFormats();
Review Comment:
nit: can use `final` here
##########
src/java/org/apache/cassandra/config/Config.java:
##########
@@ -359,9 +352,14 @@ public MemtableOptions()
public String[] data_file_directories = new String[0];
- public List<ParameterizedClass> sstable_formats = ImmutableList.of(new
ParameterizedClass(BigFormat.class.getName(),//
"org.apache.cassandra.io.sstable.format.big.BigFormat",
-
ImmutableMap.of(SSTABLE_FORMAT_ID, "0",
-
SSTABLE_FORMAT_NAME, "big")));
+ public static class SSTableFormats
+ {
+ public String selected_format;
+ public String selected_version;
+ public Map<String, Map<String, String>> options = new HashMap<>();
+ }
+
+ public SSTableFormats sstable_formats = new SSTableFormats();
Review Comment:
This doesn't seem to match the yaml format talked about in JIRA?
```
sstable:
format:
default: bti
bti:
row_index_granularity: 4KiB
```
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -1349,68 +1359,97 @@ public static void applyPartitioner(Config conf)
paritionerName = partitioner.getClass().getCanonicalName();
}
- @VisibleForTesting
- public static Map<String, Supplier<SSTableFormat<?, ?>>>
loadSSTableFormatFactories(List<ParameterizedClass> configuredFormats)
+ private static void
validateSSTableFormatFactories(Iterable<SSTableFormat.Factory> factories)
{
- ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
sstableFormatFactories =
ImmutableMap.builderWithExpectedSize(configuredFormats.size());
- Set<String> names = new HashSet<>(configuredFormats.size());
- Set<Integer> ids = new HashSet<>(configuredFormats.size());
+ Map<String, SSTableFormat.Factory> factoryByName = new HashMap<>();
+ for (SSTableFormat.Factory factory : factories)
+ {
+ if (factory.name() == null)
+ throw new ConfigurationException(String.format("SSTable format
name in %s cannot be null", factory.getClass().getName()));
+
+ if (!factory.name().matches("^[a-z]+$"))
+ throw new ConfigurationException(String.format("SSTable format
name for %s must be non-empty, lower-case letters only string",
factory.getClass().getName()));
- for (ParameterizedClass formatConfig : configuredFormats)
+ SSTableFormat.Factory prev = factoryByName.put(factory.name(),
factory);
+ if (prev != null)
+ throw new ConfigurationException(String.format("Multiple
sstable format implementations with the same name %s: %s and %s",
factory.name(), factory.getClass().getName(), prev.getClass().getName()));
+ }
+ }
+
+ private static ImmutableMap<String, Supplier<SSTableFormat<?, ?>>>
validateAndMatchSSTableFormatOptions(Iterable<SSTableFormat.Factory> factories,
Map<String, Map<String, String>> options)
+ {
+ ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
providersBuilder = ImmutableMap.builder();
+ for (SSTableFormat.Factory factory : factories)
{
- assert formatConfig.parameters != null;
- Map<String, String> params = new
HashMap<>(formatConfig.parameters);
-
- String name = params.get(Config.SSTABLE_FORMAT_NAME);
- if (name == null)
- throw new ConfigurationException("Missing 'name' parameter in
sstable format configuration for " + formatConfig.class_name);
- if (!name.matches("^[a-z]+$"))
- throw new ConfigurationException("'name' parameter in sstable
format configuration for " + formatConfig.class_name + " must be non-empty,
lower-case letters only string");
- if (names.contains(name))
- throw new ConfigurationException("Name '" + name + "' of
sstable format " + formatConfig.class_name + " is already defined for another
sstable format");
- params.remove(Config.SSTABLE_FORMAT_NAME);
-
- String idString = params.get(Config.SSTABLE_FORMAT_ID);
- if (idString == null)
- throw new ConfigurationException("Missing 'id' parameter in
sstable format configuration for " + formatConfig.class_name);
- int id;
- try
- {
- id = Integer.parseInt(idString);
- }
- catch (RuntimeException ex)
- {
- throw new ConfigurationException("'id' parameter in sstable
format configuration for " + formatConfig.class_name + " must be an integer");
- }
- if (id < 0 || id > 127)
- throw new ConfigurationException("'id' parameter in sstable
format configuration for " + formatConfig.class_name + " must be within bounds
[0..127] range");
- if (ids.contains(id))
- throw new ConfigurationException("ID '" + id + "' of sstable
format " + formatConfig.class_name + " is already defined for another sstable
format");
- params.remove(Config.SSTABLE_FORMAT_ID);
-
- Supplier<SSTableFormat<?, ?>> factory = () -> {
- Class<SSTableFormat<?, ?>> cls =
FBUtilities.classForName(formatConfig.class_name, "sstable format");
- if (!SSTableFormat.class.isAssignableFrom(cls))
- throw new ConfigurationException(String.format("Class %s
for sstable format %s does not implement %s", formatConfig.class_name, name,
SSTableFormat.class.getName()));
-
- SSTableFormat<?, ?> sstableFormat =
FBUtilities.instanceOrConstruct(cls.getName(), "sstable format");
- sstableFormat.setup(id, name, params);
- return sstableFormat;
- };
- sstableFormatFactories.put(name, factory);
- names.add(name);
- ids.add(id);
+ Map<String, String> formatOptions = options != null ?
ImmutableMap.copyOf(options.getOrDefault(factory.name(), ImmutableMap.of())) :
ImmutableMap.of();
+ providersBuilder.put(factory.name(), () ->
factory.getInstance(formatOptions));
}
+ ImmutableMap<String, Supplier<SSTableFormat<?, ?>>> providers =
providersBuilder.build();
+ if (options != null)
+ {
+ Sets.SetView<String> unknownFormatNames =
Sets.difference(options.keySet(), providers.keySet());
+ if (!unknownFormatNames.isEmpty())
+ throw new ConfigurationException(String.format("Configuration
contains options of unknown sstable formats: %s", unknownFormatNames));
+ }
+ return providers;
+ }
- return sstableFormatFactories.build();
+ private static Pair<SSTableFormat<?, ?>, Version>
getAndValidateFormatAndVersion(Map<String, SSTableFormat<?, ?>> sstableFormats,
String selectedFormatName, String selectedVersionStr)
+ {
+ SSTableFormat<?, ?> selectedFormat;
+ if (StringUtils.isBlank(selectedFormatName))
+ selectedFormatName = BigFormat.NAME;
+ selectedFormat = sstableFormats.get(selectedFormatName);
+ if (selectedFormat == null)
+ throw new ConfigurationException(String.format("Selected sstable
format '%s' is not available.", selectedFormatName));
+
+ Version selectedVersion;
+ if (StringUtils.isBlank(selectedVersionStr))
+ selectedVersion = selectedFormat.getLatestVersion();
+ else
+ selectedVersion = selectedFormat.getVersion(selectedVersionStr);
+ if (selectedVersion == null)
+ throw new ConfigurationException(String.format("Selected sstable
version '%s' is not available for the selected sstable format '%s'",
selectedVersionStr, selectedFormat.name()));
+ if (!selectedVersion.isCompatible())
+ throw new ConfigurationException(String.format("Unsupported
version '%s' for the selected sstable format '%s'", selectedVersion,
selectedFormat.name()));
+
+ return Pair.create(selectedFormat, selectedVersion);
}
private static void applySSTableFormats()
{
- if (sstableFormatFactories != null)
- logger.warn("Reinitializing SSTableFactories - this should happen
only in tests");
+ ServiceLoader<SSTableFormat.Factory> loader =
ServiceLoader.load(SSTableFormat.Factory.class,
DatabaseDescriptor.class.getClassLoader());
+ List<SSTableFormat.Factory> factories = Iterables.toList(loader);
+ if (factories.isEmpty())
+ applySSTableFormats(ImmutableList.of(new
BigFormat.BigFormatFactory()), conf.sstable_formats);
+ else
+ applySSTableFormats(factories, conf.sstable_formats);
Review Comment:
```suggestion
if (factories.isEmpty())
factories = ImmutableList.of(new BigFormat.BigFormatFactory());
applySSTableFormats(factories, conf.sstable_formats);
```
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -1349,68 +1359,97 @@ public static void applyPartitioner(Config conf)
paritionerName = partitioner.getClass().getCanonicalName();
}
- @VisibleForTesting
- public static Map<String, Supplier<SSTableFormat<?, ?>>>
loadSSTableFormatFactories(List<ParameterizedClass> configuredFormats)
+ private static void
validateSSTableFormatFactories(Iterable<SSTableFormat.Factory> factories)
{
- ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
sstableFormatFactories =
ImmutableMap.builderWithExpectedSize(configuredFormats.size());
- Set<String> names = new HashSet<>(configuredFormats.size());
- Set<Integer> ids = new HashSet<>(configuredFormats.size());
+ Map<String, SSTableFormat.Factory> factoryByName = new HashMap<>();
+ for (SSTableFormat.Factory factory : factories)
+ {
+ if (factory.name() == null)
+ throw new ConfigurationException(String.format("SSTable format
name in %s cannot be null", factory.getClass().getName()));
+
+ if (!factory.name().matches("^[a-z]+$"))
+ throw new ConfigurationException(String.format("SSTable format
name for %s must be non-empty, lower-case letters only string",
factory.getClass().getName()));
Review Comment:
```suggestion
throw new ConfigurationException(String.format("SSTable
format name for %s must be non-empty, lower-case letters only string",
factory.getClass().getCanonicalName()));
```
##########
src/java/org/apache/cassandra/io/sstable/format/SSTableFormat.java:
##########
@@ -54,9 +50,14 @@
*/
public interface SSTableFormat<R extends SSTableReader, W extends
SSTableWriter>
{
- int ordinal();
String name();
+ @Deprecated
+ default Type getType()
Review Comment:
only used for tests, and is redundant as `name` and `this.getClass()` is
known by holding reference to this
##########
test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java:
##########
@@ -198,10 +200,11 @@
"org.apache.cassandra.io.sstable.MetricsProviders",
"org.apache.cassandra.io.sstable.SSTable",
"org.apache.cassandra.io.sstable.SSTable$Builder",
- "org.apache.cassandra.io.sstable.format.AbstractSSTableFormat",
"org.apache.cassandra.io.sstable.format.SSTableFormat",
"org.apache.cassandra.io.sstable.format.SSTableFormat$Components",
"org.apache.cassandra.io.sstable.format.SSTableFormat$Components$Types",
+ "org.apache.cassandra.io.sstable.format.SSTableFormat$Factory",
+ "org.apache.cassandra.io.sstable.format.SSTableFormat$Factory",
Review Comment:
duplicate
##########
test/unit/org/apache/cassandra/schema/MockSchema.java:
##########
@@ -170,7 +170,7 @@ public static SSTableReader sstable(int generation, int
size, boolean keepRef, l
sstableId(generation),
format.getType());
- if (format == BigFormat.getInstance())
+ if (format instanceof BigFormat)
Review Comment:
I am ok with this, but the rest of the patch uses
`org.apache.cassandra.io.sstable.format.big.BigFormat#is`, which does a `name`
check
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -1349,68 +1359,97 @@ public static void applyPartitioner(Config conf)
paritionerName = partitioner.getClass().getCanonicalName();
}
- @VisibleForTesting
- public static Map<String, Supplier<SSTableFormat<?, ?>>>
loadSSTableFormatFactories(List<ParameterizedClass> configuredFormats)
+ private static void
validateSSTableFormatFactories(Iterable<SSTableFormat.Factory> factories)
{
- ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
sstableFormatFactories =
ImmutableMap.builderWithExpectedSize(configuredFormats.size());
- Set<String> names = new HashSet<>(configuredFormats.size());
- Set<Integer> ids = new HashSet<>(configuredFormats.size());
+ Map<String, SSTableFormat.Factory> factoryByName = new HashMap<>();
+ for (SSTableFormat.Factory factory : factories)
+ {
+ if (factory.name() == null)
+ throw new ConfigurationException(String.format("SSTable format
name in %s cannot be null", factory.getClass().getName()));
Review Comment:
```suggestion
throw new ConfigurationException(String.format("SSTable
format name in %s cannot be null", factory.getClass().getCanonicalName()));
```
##########
src/java/org/apache/cassandra/config/YamlConfigurationLoader.java:
##########
@@ -203,6 +202,8 @@ private static void verifyReplacements(Map<Class<?>,
Map<String, Replacement>> r
Yaml rawYaml = new Yaml(loaderOptions);
Map<String, Object> rawConfig = rawYaml.load(new
ByteArrayInputStream(configBytes));
+ if (rawConfig == null)
Review Comment:
What case caused this?
##########
src/java/org/apache/cassandra/config/Config.java:
##########
@@ -359,9 +352,14 @@ public MemtableOptions()
public String[] data_file_directories = new String[0];
- public List<ParameterizedClass> sstable_formats = ImmutableList.of(new
ParameterizedClass(BigFormat.class.getName(),//
"org.apache.cassandra.io.sstable.format.big.BigFormat",
-
ImmutableMap.of(SSTABLE_FORMAT_ID, "0",
-
SSTABLE_FORMAT_NAME, "big")));
+ public static class SSTableFormats
+ {
+ public String selected_format;
+ public String selected_version;
Review Comment:
I honestly keep debating this personally... should this be in `options`?
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -1349,68 +1359,97 @@ public static void applyPartitioner(Config conf)
paritionerName = partitioner.getClass().getCanonicalName();
}
- @VisibleForTesting
- public static Map<String, Supplier<SSTableFormat<?, ?>>>
loadSSTableFormatFactories(List<ParameterizedClass> configuredFormats)
+ private static void
validateSSTableFormatFactories(Iterable<SSTableFormat.Factory> factories)
{
- ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
sstableFormatFactories =
ImmutableMap.builderWithExpectedSize(configuredFormats.size());
- Set<String> names = new HashSet<>(configuredFormats.size());
- Set<Integer> ids = new HashSet<>(configuredFormats.size());
+ Map<String, SSTableFormat.Factory> factoryByName = new HashMap<>();
+ for (SSTableFormat.Factory factory : factories)
+ {
+ if (factory.name() == null)
+ throw new ConfigurationException(String.format("SSTable format
name in %s cannot be null", factory.getClass().getName()));
+
+ if (!factory.name().matches("^[a-z]+$"))
+ throw new ConfigurationException(String.format("SSTable format
name for %s must be non-empty, lower-case letters only string",
factory.getClass().getName()));
- for (ParameterizedClass formatConfig : configuredFormats)
+ SSTableFormat.Factory prev = factoryByName.put(factory.name(),
factory);
+ if (prev != null)
+ throw new ConfigurationException(String.format("Multiple
sstable format implementations with the same name %s: %s and %s",
factory.name(), factory.getClass().getName(), prev.getClass().getName()));
Review Comment:
```suggestion
throw new ConfigurationException(String.format("Multiple
sstable format implementations with the same name %s: %s and %s",
factory.name(), factory.getClass().getCanonicalName(),
prev.getClass().getCanonicalName()));
```
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -1349,68 +1359,97 @@ public static void applyPartitioner(Config conf)
paritionerName = partitioner.getClass().getCanonicalName();
}
- @VisibleForTesting
- public static Map<String, Supplier<SSTableFormat<?, ?>>>
loadSSTableFormatFactories(List<ParameterizedClass> configuredFormats)
+ private static void
validateSSTableFormatFactories(Iterable<SSTableFormat.Factory> factories)
{
- ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
sstableFormatFactories =
ImmutableMap.builderWithExpectedSize(configuredFormats.size());
- Set<String> names = new HashSet<>(configuredFormats.size());
- Set<Integer> ids = new HashSet<>(configuredFormats.size());
+ Map<String, SSTableFormat.Factory> factoryByName = new HashMap<>();
+ for (SSTableFormat.Factory factory : factories)
+ {
+ if (factory.name() == null)
+ throw new ConfigurationException(String.format("SSTable format
name in %s cannot be null", factory.getClass().getName()));
+
+ if (!factory.name().matches("^[a-z]+$"))
+ throw new ConfigurationException(String.format("SSTable format
name for %s must be non-empty, lower-case letters only string",
factory.getClass().getName()));
- for (ParameterizedClass formatConfig : configuredFormats)
+ SSTableFormat.Factory prev = factoryByName.put(factory.name(),
factory);
+ if (prev != null)
+ throw new ConfigurationException(String.format("Multiple
sstable format implementations with the same name %s: %s and %s",
factory.name(), factory.getClass().getName(), prev.getClass().getName()));
+ }
+ }
+
+ private static ImmutableMap<String, Supplier<SSTableFormat<?, ?>>>
validateAndMatchSSTableFormatOptions(Iterable<SSTableFormat.Factory> factories,
Map<String, Map<String, String>> options)
+ {
+ ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
providersBuilder = ImmutableMap.builder();
+ for (SSTableFormat.Factory factory : factories)
{
- assert formatConfig.parameters != null;
- Map<String, String> params = new
HashMap<>(formatConfig.parameters);
-
- String name = params.get(Config.SSTABLE_FORMAT_NAME);
- if (name == null)
- throw new ConfigurationException("Missing 'name' parameter in
sstable format configuration for " + formatConfig.class_name);
- if (!name.matches("^[a-z]+$"))
- throw new ConfigurationException("'name' parameter in sstable
format configuration for " + formatConfig.class_name + " must be non-empty,
lower-case letters only string");
- if (names.contains(name))
- throw new ConfigurationException("Name '" + name + "' of
sstable format " + formatConfig.class_name + " is already defined for another
sstable format");
- params.remove(Config.SSTABLE_FORMAT_NAME);
-
- String idString = params.get(Config.SSTABLE_FORMAT_ID);
- if (idString == null)
- throw new ConfigurationException("Missing 'id' parameter in
sstable format configuration for " + formatConfig.class_name);
- int id;
- try
- {
- id = Integer.parseInt(idString);
- }
- catch (RuntimeException ex)
- {
- throw new ConfigurationException("'id' parameter in sstable
format configuration for " + formatConfig.class_name + " must be an integer");
- }
- if (id < 0 || id > 127)
- throw new ConfigurationException("'id' parameter in sstable
format configuration for " + formatConfig.class_name + " must be within bounds
[0..127] range");
- if (ids.contains(id))
- throw new ConfigurationException("ID '" + id + "' of sstable
format " + formatConfig.class_name + " is already defined for another sstable
format");
- params.remove(Config.SSTABLE_FORMAT_ID);
-
- Supplier<SSTableFormat<?, ?>> factory = () -> {
- Class<SSTableFormat<?, ?>> cls =
FBUtilities.classForName(formatConfig.class_name, "sstable format");
- if (!SSTableFormat.class.isAssignableFrom(cls))
- throw new ConfigurationException(String.format("Class %s
for sstable format %s does not implement %s", formatConfig.class_name, name,
SSTableFormat.class.getName()));
-
- SSTableFormat<?, ?> sstableFormat =
FBUtilities.instanceOrConstruct(cls.getName(), "sstable format");
- sstableFormat.setup(id, name, params);
- return sstableFormat;
- };
- sstableFormatFactories.put(name, factory);
- names.add(name);
- ids.add(id);
+ Map<String, String> formatOptions = options != null ?
ImmutableMap.copyOf(options.getOrDefault(factory.name(), ImmutableMap.of())) :
ImmutableMap.of();
Review Comment:
rather than checking `options` in the loop you can do the following outside
the loop
```
if (options == null)
options = ImmutableMap.of();
```
##########
test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java:
##########
@@ -217,6 +220,8 @@
"org.apache.cassandra.io.sstable.format.SortedTableWriter$Builder",
"org.apache.cassandra.io.sstable.format.Version",
"org.apache.cassandra.io.sstable.format.big.BigFormat",
+ "org.apache.cassandra.io.sstable.format.big.BigFormat$BigFormatFactory",
+ "org.apache.cassandra.io.sstable.format.big.BigFormat$BigFormatFactory",
Review Comment:
duplicate
##########
src/java/org/apache/cassandra/io/sstable/format/SSTableFormat.java:
##########
@@ -113,99 +114,85 @@ IScrubber getScrubber(ColumnFamilyStore cfs,
void deleteOrphanedComponents(Descriptor descriptor, Set<Component>
components);
- void setup(int id, String name, Map<String, String> options);
-
- Type getType();
-
/**
* Deletes the existing components of the sstables represented by the
provided descriptor.
* The method is also responsible for cleaning up the in-memory resources
occupied by the stuff related to that
* sstables, such as row key cache entries.
*/
void delete(Descriptor descriptor);
+ /**
+ * This class is not completely redundant
Review Comment:
I do feel that this is redundant, its just another reference to the values
in `org.apache.cassandra.config.DatabaseDescriptor#sstableFormats` and 100%
depends on that...
`current` -> `DD.getSelectedSSTableFormat`
`getByName` -> `DD.sstableFormats.get(name)`
`getByClass` ->
```
DatabaseDescriptor.getSSTableFormats().values().stream()
.filter(f -> f.getClass().equals(formatClass))
.findFirst()
.orElseGet(() -> {
throw new NoSuchElementException("Unknown
sstable format class: " + formatClass);
});
```
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -1349,68 +1359,97 @@ public static void applyPartitioner(Config conf)
paritionerName = partitioner.getClass().getCanonicalName();
}
- @VisibleForTesting
- public static Map<String, Supplier<SSTableFormat<?, ?>>>
loadSSTableFormatFactories(List<ParameterizedClass> configuredFormats)
+ private static void
validateSSTableFormatFactories(Iterable<SSTableFormat.Factory> factories)
{
- ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
sstableFormatFactories =
ImmutableMap.builderWithExpectedSize(configuredFormats.size());
- Set<String> names = new HashSet<>(configuredFormats.size());
- Set<Integer> ids = new HashSet<>(configuredFormats.size());
+ Map<String, SSTableFormat.Factory> factoryByName = new HashMap<>();
+ for (SSTableFormat.Factory factory : factories)
+ {
+ if (factory.name() == null)
+ throw new ConfigurationException(String.format("SSTable format
name in %s cannot be null", factory.getClass().getName()));
+
+ if (!factory.name().matches("^[a-z]+$"))
+ throw new ConfigurationException(String.format("SSTable format
name for %s must be non-empty, lower-case letters only string",
factory.getClass().getName()));
- for (ParameterizedClass formatConfig : configuredFormats)
+ SSTableFormat.Factory prev = factoryByName.put(factory.name(),
factory);
+ if (prev != null)
+ throw new ConfigurationException(String.format("Multiple
sstable format implementations with the same name %s: %s and %s",
factory.name(), factory.getClass().getName(), prev.getClass().getName()));
+ }
+ }
+
+ private static ImmutableMap<String, Supplier<SSTableFormat<?, ?>>>
validateAndMatchSSTableFormatOptions(Iterable<SSTableFormat.Factory> factories,
Map<String, Map<String, String>> options)
+ {
+ ImmutableMap.Builder<String, Supplier<SSTableFormat<?, ?>>>
providersBuilder = ImmutableMap.builder();
+ for (SSTableFormat.Factory factory : factories)
{
- assert formatConfig.parameters != null;
- Map<String, String> params = new
HashMap<>(formatConfig.parameters);
-
- String name = params.get(Config.SSTABLE_FORMAT_NAME);
- if (name == null)
- throw new ConfigurationException("Missing 'name' parameter in
sstable format configuration for " + formatConfig.class_name);
- if (!name.matches("^[a-z]+$"))
- throw new ConfigurationException("'name' parameter in sstable
format configuration for " + formatConfig.class_name + " must be non-empty,
lower-case letters only string");
- if (names.contains(name))
- throw new ConfigurationException("Name '" + name + "' of
sstable format " + formatConfig.class_name + " is already defined for another
sstable format");
- params.remove(Config.SSTABLE_FORMAT_NAME);
-
- String idString = params.get(Config.SSTABLE_FORMAT_ID);
- if (idString == null)
- throw new ConfigurationException("Missing 'id' parameter in
sstable format configuration for " + formatConfig.class_name);
- int id;
- try
- {
- id = Integer.parseInt(idString);
- }
- catch (RuntimeException ex)
- {
- throw new ConfigurationException("'id' parameter in sstable
format configuration for " + formatConfig.class_name + " must be an integer");
- }
- if (id < 0 || id > 127)
- throw new ConfigurationException("'id' parameter in sstable
format configuration for " + formatConfig.class_name + " must be within bounds
[0..127] range");
- if (ids.contains(id))
- throw new ConfigurationException("ID '" + id + "' of sstable
format " + formatConfig.class_name + " is already defined for another sstable
format");
- params.remove(Config.SSTABLE_FORMAT_ID);
-
- Supplier<SSTableFormat<?, ?>> factory = () -> {
- Class<SSTableFormat<?, ?>> cls =
FBUtilities.classForName(formatConfig.class_name, "sstable format");
- if (!SSTableFormat.class.isAssignableFrom(cls))
- throw new ConfigurationException(String.format("Class %s
for sstable format %s does not implement %s", formatConfig.class_name, name,
SSTableFormat.class.getName()));
-
- SSTableFormat<?, ?> sstableFormat =
FBUtilities.instanceOrConstruct(cls.getName(), "sstable format");
- sstableFormat.setup(id, name, params);
- return sstableFormat;
- };
- sstableFormatFactories.put(name, factory);
- names.add(name);
- ids.add(id);
+ Map<String, String> formatOptions = options != null ?
ImmutableMap.copyOf(options.getOrDefault(factory.name(), ImmutableMap.of())) :
ImmutableMap.of();
+ providersBuilder.put(factory.name(), () ->
factory.getInstance(formatOptions));
}
+ ImmutableMap<String, Supplier<SSTableFormat<?, ?>>> providers =
providersBuilder.build();
+ if (options != null)
+ {
+ Sets.SetView<String> unknownFormatNames =
Sets.difference(options.keySet(), providers.keySet());
+ if (!unknownFormatNames.isEmpty())
+ throw new ConfigurationException(String.format("Configuration
contains options of unknown sstable formats: %s", unknownFormatNames));
+ }
+ return providers;
+ }
- return sstableFormatFactories.build();
+ private static Pair<SSTableFormat<?, ?>, Version>
getAndValidateFormatAndVersion(Map<String, SSTableFormat<?, ?>> sstableFormats,
String selectedFormatName, String selectedVersionStr)
+ {
+ SSTableFormat<?, ?> selectedFormat;
+ if (StringUtils.isBlank(selectedFormatName))
+ selectedFormatName = BigFormat.NAME;
Review Comment:
I prefer this to live in `Config` so defaults are more clear and not
scattered all over the place
--
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]
