Github user ivakegg commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/253#discussion_r129966848
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java
---
@@ -39,73 +40,139 @@
/**
* A {@link RandomVolumeChooser} that limits its choices from a given set
of options to the subset of those options preferred for a particular table.
Defaults
- * to selecting from all of the options presented. Can be customized via
the table property {@value #PREFERRED_VOLUMES_CUSTOM_KEY}, which should contain
a comma
+ * to selecting from all of the options presented. Can be customized via
the table property table.custom.preferredVolumes, which should contain a comma
* separated list of {@link Volume} URIs. Note that both the property name
and the format of its value are specific to this particular implementation.
*/
public class PreferredVolumeChooser extends RandomVolumeChooser {
private static final Logger log =
LoggerFactory.getLogger(PreferredVolumeChooser.class);
- /**
- * This should match {@link Property#TABLE_ARBITRARY_PROP_PREFIX}
- */
- public static final String PREFERRED_VOLUMES_CUSTOM_KEY =
"table.custom.preferredVolumes";
- // TODO ACCUMULO-3417 replace this with the ability to retrieve by
String key.
- private static final Predicate<String> PREFERRED_VOLUMES_FILTER = key ->
PREFERRED_VOLUMES_CUSTOM_KEY.equals(key);
+ public static final String TABLE_PREFERRED_VOLUMES =
Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "preferred.volumes";
+
+ public static final String SCOPED_PREFERRED_VOLUMES(String scope) {
+ return Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + scope +
".preferred.volumes";
+ }
+
+ public static final String DEFAULT_SCOPED_PREFERRED_VOLUMES =
SCOPED_PREFERRED_VOLUMES("scoped");
@SuppressWarnings("unchecked")
private final Map<String,Set<String>> parsedPreferredVolumes =
Collections.synchronizedMap(new LRUMap(1000));
// TODO has to be lazily initialized currently because of the reliance
on HdfsZooInstance. see ACCUMULO-3411
private volatile ServerConfigurationFactory serverConfs;
@Override
- public String choose(VolumeChooserEnvironment env, String[] options) {
- if (!env.hasTableId())
+ public String choose(VolumeChooserEnvironment env, String[] options)
throws AccumuloException {
+ if (!env.hasTableId() && !env.hasScope()) {
+ // this should only happen during initialize
+ log.warn("No table id or scope, so it's not possible to determine
preferred volumes. Using all volumes.");
return super.choose(env, options);
+ }
- // Get the current table's properties, and find the preferred volumes
property
- // This local variable is an intentional component of the single-check
idiom.
- ServerConfigurationFactory localConf = serverConfs;
- if (localConf == null) {
- // If we're under contention when first getting here we'll throw
away some initializations.
- localConf = new
ServerConfigurationFactory(HdfsZooInstance.getInstance());
- serverConfs = localConf;
+ // get the volumes property
+ ServerConfigurationFactory localConf = loadConf();
+ List<String> volumes;
+ if (env.hasTableId()) {
+ volumes = getPreferredVolumesForTable(env, localConf, options);
+ //
localConf.getTableConfiguration(env.getTableId()).get(PREFERRED_VOLUMES_CUSTOM_KEY);
+ } else { // env.hasScope()
+ volumes = getPreferredVolumesForNonTable(env, localConf, options);
+ //
localConf.getSystemConfiguration().get(PREFERRED_VOLUMES_SCOPED_KEY(env.getScope()));
}
- TableConfiguration tableConf =
localConf.getTableConfiguration(env.getTableId());
- final Map<String,String> props = new HashMap<>();
- tableConf.getProperties(props, PREFERRED_VOLUMES_FILTER);
- if (props.isEmpty()) {
- log.warn("No preferred volumes specified. Defaulting to randomly
choosing from instance volumes");
- return super.choose(env, options);
+
+ // Randomly choose the volume from the preferred volumes
+ String choice = super.choose(env, volumes.toArray(EMPTY_STRING_ARRAY));
+ if (log.isTraceEnabled()) {
+ log.trace("Choice = " + choice);
}
- String volumes = props.get(PREFERRED_VOLUMES_CUSTOM_KEY);
+ return choice;
+ }
+ private List<String>
getPreferredVolumesForTable(VolumeChooserEnvironment env,
ServerConfigurationFactory localConf, String[] options)
+ throws AccumuloException {
if (log.isTraceEnabled()) {
- log.trace("In custom chooser");
- log.trace("Volumes: " + volumes);
- log.trace("TableID: " + env.getTableId());
+ log.trace("Looking up property " + TABLE_PREFERRED_VOLUMES + " for
Table id: " + env.getTableId());
}
- // If the preferred volumes property was specified, split the returned
string by the comma and add use it to filter the given options.
- Set<String> preferred = parsedPreferredVolumes.get(volumes);
- if (preferred == null) {
- preferred = new HashSet<>(Arrays.asList(StringUtils.split(volumes,
',')));
- parsedPreferredVolumes.put(volumes, preferred);
+ final TableConfiguration tableConf =
localConf.getTableConfiguration(env.getTableId());
+ String volumes = tableConf.get(TABLE_PREFERRED_VOLUMES);
+
+ return getFilteredOptions(TABLE_PREFERRED_VOLUMES, volumes, options);
+ }
+
+ private List<String>
getPreferredVolumesForNonTable(VolumeChooserEnvironment env,
ServerConfigurationFactory localConf, String[] options)
+ throws AccumuloException {
+ String property = SCOPED_PREFERRED_VOLUMES(env.getScope());
+
+ if (log.isTraceEnabled()) {
+ log.trace("Looking up property: " + property);
+ }
+
+ AccumuloConfiguration systemConfiguration =
localConf.getSystemConfiguration();
+ String volumes = systemConfiguration.get(property);
+
+ // only if the custom property is not set to we fallback to the
default scoped preferred volumes
+ if (null == volumes) {
+ log.debug("Property not found: " + property + " using " +
DEFAULT_SCOPED_PREFERRED_VOLUMES);
+ property = DEFAULT_SCOPED_PREFERRED_VOLUMES;
+ volumes = systemConfiguration.get(DEFAULT_SCOPED_PREFERRED_VOLUMES);
--- End diff --
If we want this to work like the table preferred volume where you can set a
default in the system configuration, the we would need to have scoped
configurations like we have table specific configurations. I am not sure how
to handle this in the same way. We could get rid of the default scope property
but then we would be requiring users to set both the "logger" and "init" scoped
properties which seems wrong. Realistically users will only set the default
one until we actually come up with other scopes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
