[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1100625103
##
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##
@@ -120,11 +120,11 @@ public enum Property {
+ " a comma or other reserved characters in a URI use standard URI
hex"
+ " encoding. For example replace commas with %2C.",
"1.6.0"),
- INSTANCE_VOLUMES_CONFIG("instance.volumes.config.", null,
PropertyType.PREFIX,
+ INSTANCE_VOLUMES_CONFIG("instance.volume.config.", null, PropertyType.PREFIX,
Review Comment:
Changes applied in 55fe4f3
##
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java:
##
@@ -382,44 +382,37 @@ public short getDefaultReplication(Path path) {
*/
private static Configuration
getVolumeManagerConfiguration(AccumuloConfiguration conf,
final Configuration hadoopConf, final String filesystemURI) {
+
final Configuration volumeConfig = new Configuration(hadoopConf);
-final Map customProps =
-
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG);
-customProps.forEach((key, value) -> {
- if (key.startsWith(filesystemURI)) {
-String property = key.substring(filesystemURI.length() + 1);
-log.info("Overriding property {} to {} for volume {}", property,
value, filesystemURI);
-volumeConfig.set(property, value);
- }
-});
-return volumeConfig;
- }
- private static void warnVolumeOverridesMissingVolume(AccumuloConfiguration
conf,
- Set definedVolumes) {
-final Map overrideProperties = new ConcurrentHashMap<>(
-
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG));
+
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG).entrySet().stream()
+.filter(e -> e.getKey().startsWith(filesystemURI + ".")).forEach(e -> {
+ String key = e.getKey().substring(filesystemURI.length() + 1);
+ String value = e.getValue();
+ log.info("Overriding property {} for volume {}", key, value,
filesystemURI);
+ volumeConfig.set(key, value);
+});
-definedVolumes.forEach(vol -> {
- log.debug("Looking for defined volume: {}", vol);
- overrideProperties.keySet().forEach(override -> {
-if (override.startsWith(vol)) {
- log.debug("Found volume {}, removing property {}", vol, override);
- overrideProperties.remove(override);
-}
- });
-});
+return volumeConfig;
+ }
-overrideProperties.forEach((k, v) -> log
-.warn("Found no matching volume for volume config override property {}
= {}", k, v));
+ protected static List>
+ findVolumeOverridesMissingVolume(AccumuloConfiguration conf, Set
definedVolumes) {
+return
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG).entrySet()
+.stream()
+// log only configs where none of the volumes (with a dot) prefix its
key
+.filter(e -> definedVolumes.stream().noneMatch(vol ->
e.getKey().startsWith(vol + ".")))
+.collect(Collectors.toList());
Review Comment:
Changes applied in 55fe4f3
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1099087530
##
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java:
##
@@ -353,11 +363,63 @@ public short getDefaultReplication(Path path) {
return getFileSystemByPath(path).getDefaultReplication(path);
}
+ /**
+ * The Hadoop Configuration object does not currently allow for duplicate
properties to be set in
+ * a single Configuration for different FileSystem URIs. Here we will look
for properties in the
+ * Accumulo configuration of the form:
+ *
+ *
+ * instance.volume.config.volume-uri.hdfs-property
+ *
+ *
+ * We will use these properties to return a new Configuration object that
can be used with the
+ * FileSystem URI.
+ *
+ * @param conf AccumuloConfiguration object
+ * @param hadoopConf Hadoop Configuration object
+ * @param filesystemURI Volume Filesystem URI
+ * @return Hadoop Configuration with custom overrides for this FileSystem
+ */
+ private static Configuration
getVolumeManagerConfiguration(AccumuloConfiguration conf,
+ final Configuration hadoopConf, final String filesystemURI) {
+final Configuration volumeConfig = new Configuration(hadoopConf);
+final Map customProps =
+
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG);
+customProps.forEach((key, value) -> {
+ if (key.startsWith(filesystemURI)) {
+String property = key.substring(filesystemURI.length() + 1);
+log.info("Overriding property {} to {} for volume {}", property,
value, filesystemURI);
+volumeConfig.set(property, value);
+ }
+});
Review Comment:
Addressed in f34eb45.
##
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloConfigImpl.java:
##
@@ -812,4 +820,29 @@ public int getNumCompactors() {
public void setNumCompactors(int numCompactors) {
this.numCompactors = numCompactors;
}
+
+ /**
+ * Set the FinalSiteConfigUpdater instance that will be used to modify the
site configuration
+ * right before it's written out a file. This would be useful in the case
where the configuration
+ * needs to be updated based on a property that is set in
MiniAccumuloClusterImpl like
+ * instance.volumes
+ *
+ * @param updater FinalSiteConfigUpdater instance
+ * @since 2.1.1
+ */
+ public void setFinalSiteConfigUpdater(FinalSiteConfigUpdater updater) {
Review Comment:
Addressed in f34eb45.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1099087785
##
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloConfigImpl.java:
##
@@ -47,6 +47,12 @@
* @since 1.6.0
*/
public class MiniAccumuloConfigImpl {
+
+ @FunctionalInterface
+ public static interface FinalSiteConfigUpdater {
+void update(MiniAccumuloConfigImpl cfg);
+ }
Review Comment:
Addressed in f34eb45.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1099087330
##
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java:
##
@@ -353,11 +363,63 @@ public short getDefaultReplication(Path path) {
return getFileSystemByPath(path).getDefaultReplication(path);
}
+ /**
+ * The Hadoop Configuration object does not currently allow for duplicate
properties to be set in
+ * a single Configuration for different FileSystem URIs. Here we will look
for properties in the
+ * Accumulo configuration of the form:
+ *
+ *
+ * instance.volume.config.volume-uri.hdfs-property
+ *
+ *
+ * We will use these properties to return a new Configuration object that
can be used with the
+ * FileSystem URI.
+ *
+ * @param conf AccumuloConfiguration object
+ * @param hadoopConf Hadoop Configuration object
+ * @param filesystemURI Volume Filesystem URI
+ * @return Hadoop Configuration with custom overrides for this FileSystem
+ */
+ private static Configuration
getVolumeManagerConfiguration(AccumuloConfiguration conf,
+ final Configuration hadoopConf, final String filesystemURI) {
+final Configuration volumeConfig = new Configuration(hadoopConf);
+final Map customProps =
+
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG);
+customProps.forEach((key, value) -> {
+ if (key.startsWith(filesystemURI)) {
+String property = key.substring(filesystemURI.length() + 1);
+log.info("Overriding property {} to {} for volume {}", property,
value, filesystemURI);
+volumeConfig.set(property, value);
+ }
+});
+return volumeConfig;
+ }
+
+ private static void warnVolumeOverridesMissingVolume(AccumuloConfiguration
conf,
+ Set definedVolumes) {
+final Map overrideProperties = new ConcurrentHashMap<>(
+
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG));
+
+definedVolumes.forEach(vol -> {
+ log.debug("Looking for defined volume: {}", vol);
+ overrideProperties.keySet().forEach(override -> {
+if (override.startsWith(vol)) {
+ log.debug("Found volume {}, removing property {}", vol, override);
+ overrideProperties.remove(override);
+}
+ });
+});
+
+overrideProperties.forEach((k, v) -> log
+.warn("Found no matching volume for volume config override property {}
= {}", k, v));
+ }
Review Comment:
Addressed in f34eb45.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1099086976
##
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloConfigImpl.java:
##
@@ -812,4 +820,29 @@ public int getNumCompactors() {
public void setNumCompactors(int numCompactors) {
this.numCompactors = numCompactors;
}
+
+ /**
+ * Set the FinalSiteConfigUpdater instance that will be used to modify the
site configuration
+ * right before it's written out a file. This would be useful in the case
where the configuration
+ * needs to be updated based on a property that is set in
MiniAccumuloClusterImpl like
+ * instance.volumes
+ *
+ * @param updater FinalSiteConfigUpdater instance
+ * @since 2.1.1
+ */
+ public void setFinalSiteConfigUpdater(FinalSiteConfigUpdater updater) {
+this.updater = updater;
+ }
+
+ /**
+ * Called by MiniAccumuloClusterImpl after all modifications are done to the
configuration and
+ * right before it's written out to a file.
+ *
+ * @since 2.1.1
Review Comment:
Addressed in f34eb45.
##
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java:
##
@@ -353,11 +363,63 @@ public short getDefaultReplication(Path path) {
return getFileSystemByPath(path).getDefaultReplication(path);
}
+ /**
+ * The Hadoop Configuration object does not currently allow for duplicate
properties to be set in
+ * a single Configuration for different FileSystem URIs. Here we will look
for properties in the
+ * Accumulo configuration of the form:
+ *
+ *
+ * instance.volume.config.volume-uri.hdfs-property
+ *
+ *
+ * We will use these properties to return a new Configuration object that
can be used with the
+ * FileSystem URI.
+ *
+ * @param conf AccumuloConfiguration object
+ * @param hadoopConf Hadoop Configuration object
+ * @param filesystemURI Volume Filesystem URI
+ * @return Hadoop Configuration with custom overrides for this FileSystem
+ */
+ private static Configuration
getVolumeManagerConfiguration(AccumuloConfiguration conf,
+ final Configuration hadoopConf, final String filesystemURI) {
+final Configuration volumeConfig = new Configuration(hadoopConf);
+final Map customProps =
+
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG);
+customProps.forEach((key, value) -> {
+ if (key.startsWith(filesystemURI)) {
+String property = key.substring(filesystemURI.length() + 1);
+log.info("Overriding property {} to {} for volume {}", property,
value, filesystemURI);
+volumeConfig.set(property, value);
+ }
+});
+return volumeConfig;
+ }
+
+ private static void warnVolumeOverridesMissingVolume(AccumuloConfiguration
conf,
+ Set definedVolumes) {
+final Map overrideProperties = new ConcurrentHashMap<>(
+
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG));
+
+definedVolumes.forEach(vol -> {
+ log.debug("Looking for defined volume: {}", vol);
+ overrideProperties.keySet().forEach(override -> {
+if (override.startsWith(vol)) {
+ log.debug("Found volume {}, removing property {}", vol, override);
+ overrideProperties.remove(override);
+}
+ });
+});
Review Comment:
Addressed in f34eb45.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1099086779
##
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##
@@ -120,6 +120,16 @@ public enum Property {
+ " a comma or other reserved characters in a URI use standard URI
hex"
+ " encoding. For example replace commas with %2C.",
"1.6.0"),
+ INSTANCE_VOLUMES_CONFIG("instance.volumes.config.", null,
PropertyType.PREFIX,
+ "Properties in this category are used to provide volume specific
overrides to "
+ + "the general filesystem client configuration. Properties using
this prefix "
+ + "should be in the form "
+ +
"'instance.volumes.config..=. An "
+ + "example: "
+ +
"'instance.volume.config.hdfs://namespace-a:8020/accumulo.dfs.client.hedged.read.threadpool.size=10'.
"
+ + "Note that when specifying property names that contain colons in
the properties "
+ + "files that the colons need to be escaped with a backslash.",
+ "2.1.1"),
Review Comment:
Addressed in f34eb45.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1099086326
##
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java:
##
@@ -353,11 +363,63 @@ public short getDefaultReplication(Path path) {
return getFileSystemByPath(path).getDefaultReplication(path);
}
+ /**
+ * The Hadoop Configuration object does not currently allow for duplicate
properties to be set in
+ * a single Configuration for different FileSystem URIs. Here we will look
for properties in the
+ * Accumulo configuration of the form:
+ *
+ *
+ * instance.volume.config.volume-uri.hdfs-property
+ *
+ *
+ * We will use these properties to return a new Configuration object that
can be used with the
+ * FileSystem URI.
+ *
+ * @param conf AccumuloConfiguration object
+ * @param hadoopConf Hadoop Configuration object
+ * @param filesystemURI Volume Filesystem URI
+ * @return Hadoop Configuration with custom overrides for this FileSystem
+ */
+ private static Configuration
getVolumeManagerConfiguration(AccumuloConfiguration conf,
+ final Configuration hadoopConf, final String filesystemURI) {
+final Configuration volumeConfig = new Configuration(hadoopConf);
+final Map customProps =
+
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG);
+customProps.forEach((key, value) -> {
+ if (key.startsWith(filesystemURI)) {
+String property = key.substring(filesystemURI.length() + 1);
+log.info("Overriding property {} to {} for volume {}", property,
value, filesystemURI);
+volumeConfig.set(property, value);
+ }
+});
+return volumeConfig;
+ }
+
+ private static void warnVolumeOverridesMissingVolume(AccumuloConfiguration
conf,
+ Set definedVolumes) {
+final Map overrideProperties = new ConcurrentHashMap<>(
+
conf.getAllPropertiesWithPrefixStripped(Property.INSTANCE_VOLUMES_CONFIG));
+
+definedVolumes.forEach(vol -> {
+ log.debug("Looking for defined volume: {}", vol);
+ overrideProperties.keySet().forEach(override -> {
+if (override.startsWith(vol)) {
+ log.debug("Found volume {}, removing property {}", vol, override);
+ overrideProperties.remove(override);
+}
+ });
+});
+
+overrideProperties.forEach((k, v) -> log
+.warn("Found no matching volume for volume config override property {}
= {}", k, v));
Review Comment:
Implemented test in f34eb45.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1097416330
##
server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java:
##
@@ -104,4 +111,88 @@ public Scope getChooserScope() {
assertThrows(RuntimeException.class, () -> vm.choose(chooserEnv,
volumes));
}
}
+
+ @Test
+ public void testConfigurationOverrides() throws Exception {
+
+final String vol1 = "file://127.0.0.1/vol1/";
+final String vol2 = "file://localhost/vol2/";
+final String vol3 = "hdfs://127.0.0.1/accumulo";
+final String vol4 = "hdfs://localhost/accumulo";
+
+ConfigurationCopy conf = new ConfigurationCopy();
+conf.set(Property.INSTANCE_VOLUMES, String.join(",", vol1, vol2, vol3,
vol4));
+conf.set(Property.GENERAL_VOLUME_CHOOSER,
Property.GENERAL_VOLUME_CHOOSER.getDefaultValue());
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol1 + "."
++ HdfsClientConfigKeys.HedgedRead.THREADPOOL_SIZE_KEY, "10");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol1 + "."
Review Comment:
Added blocksize property to one volume in test.
##
server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java:
##
@@ -104,4 +111,88 @@ public Scope getChooserScope() {
assertThrows(RuntimeException.class, () -> vm.choose(chooserEnv,
volumes));
}
}
+
+ @Test
+ public void testConfigurationOverrides() throws Exception {
+
+final String vol1 = "file://127.0.0.1/vol1/";
+final String vol2 = "file://localhost/vol2/";
+final String vol3 = "hdfs://127.0.0.1/accumulo";
+final String vol4 = "hdfs://localhost/accumulo";
+
+ConfigurationCopy conf = new ConfigurationCopy();
+conf.set(Property.INSTANCE_VOLUMES, String.join(",", vol1, vol2, vol3,
vol4));
+conf.set(Property.GENERAL_VOLUME_CHOOSER,
Property.GENERAL_VOLUME_CHOOSER.getDefaultValue());
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol1 + "."
++ HdfsClientConfigKeys.HedgedRead.THREADPOOL_SIZE_KEY, "10");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol1 + "."
Review Comment:
Added blocksize property to one volume in test in e0c12c4.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1097415819
##
server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java:
##
@@ -104,4 +111,88 @@ public Scope getChooserScope() {
assertThrows(RuntimeException.class, () -> vm.choose(chooserEnv,
volumes));
}
}
+
+ @Test
+ public void testConfigurationOverrides() throws Exception {
+
+final String vol1 = "file://127.0.0.1/vol1/";
+final String vol2 = "file://localhost/vol2/";
+final String vol3 = "hdfs://127.0.0.1/accumulo";
+final String vol4 = "hdfs://localhost/accumulo";
+
+ConfigurationCopy conf = new ConfigurationCopy();
+conf.set(Property.INSTANCE_VOLUMES, String.join(",", vol1, vol2, vol3,
vol4));
+conf.set(Property.GENERAL_VOLUME_CHOOSER,
Property.GENERAL_VOLUME_CHOOSER.getDefaultValue());
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol1 + "."
++ HdfsClientConfigKeys.HedgedRead.THREADPOOL_SIZE_KEY, "10");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol1 + "."
++ HdfsClientConfigKeys.DFS_CLIENT_CACHE_DROP_BEHIND_READS, "true");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol2 + "."
++ HdfsClientConfigKeys.HedgedRead.THREADPOOL_SIZE_KEY, "20");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol2 + "."
++ HdfsClientConfigKeys.DFS_CLIENT_CACHE_DROP_BEHIND_READS, "false");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol3 + "."
++ HdfsClientConfigKeys.HedgedRead.THREADPOOL_SIZE_KEY, "30");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol3 + "."
++ HdfsClientConfigKeys.DFS_CLIENT_CACHE_DROP_BEHIND_READS, "TRUE");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol4 + "."
++ HdfsClientConfigKeys.HedgedRead.THREADPOOL_SIZE_KEY, "40");
+conf.set(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol4 + "."
++ HdfsClientConfigKeys.DFS_CLIENT_CACHE_DROP_BEHIND_READS, "FALSE");
+
+VolumeManager vm = VolumeManagerImpl.get(conf, hadoopConf);
+
+FileSystem fs1 = vm.getFileSystemByPath(new Path(vol1));
+Configuration conf1 = fs1.getConf();
+
+FileSystem fs2 = vm.getFileSystemByPath(new Path(vol2));
+Configuration conf2 = fs2.getConf();
+
+FileSystem fs3 = vm.getFileSystemByPath(new Path(vol3));
+Configuration conf3 = fs3.getConf();
+
+FileSystem fs4 = vm.getFileSystemByPath(new Path(vol4));
+Configuration conf4 = fs4.getConf();
Review Comment:
Added a fifth volume to the test in e0c12c4.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1097415535
##
test/src/main/java/org/apache/accumulo/test/VolumeManagerIT.java:
##
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.client.Accumulo;
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.admin.NewTableConfiguration;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacBase;
+import org.apache.accumulo.test.functional.ReadWriteIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.junit.jupiter.api.Test;
+
+public class VolumeManagerIT extends ConfigurableMacBase {
+
+ private String vol1;
+ private String vol2;
+
+ @Override
+ protected void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+cfg.useMiniDFS(true);
+cfg.setFinalSiteConfigUpdater((config) -> {
+ vol1 = config.getSiteConfig().get(Property.INSTANCE_VOLUMES.getKey());
+ assertTrue(vol1.contains("localhost"));
+ vol2 = vol1.replace("localhost", "127.0.0.1");
+ config.setProperty(Property.INSTANCE_VOLUMES.getKey(), String.join(",",
vol2, vol1));
+
+ // Set Volume specific HDFS overrides
+ config.setProperty("general.volume.chooser",
+ "org.apache.accumulo.core.spi.fs.PreferredVolumeChooser");
+ config.setProperty("general.custom.volume.preferred.default", vol1);
+ config.setProperty("general.custom.volume.preferred.logger", vol2);
+
+ // Need to escape the colons in the custom property volume because it's
part of the key. It's
+ // being
+ // written out to a file and read in using the Properties object.
+ String vol1FileOutput = vol1.replaceAll(":", ":");
+ String vol2FileOutput = vol2.replaceAll(":", ":");
+ config.setProperty(
+ Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol1FileOutput +
".dfs.blocksize",
+ "10485760");
+ config.setProperty(
+ Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol2FileOutput +
".dfs.blocksize",
+ "104857600");
+ config.setProperty(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() +
vol1FileOutput
+ + ".dfs.client.use.datanode.hostname", "true");
+ config.setProperty(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() +
vol2FileOutput
+ + ".dfs.client.use.datanode.hostname", "false");
+ config.setProperty(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() +
vol1FileOutput
+ + ".dfs.client.hedged.read.threadpool.size", "0");
+ config.setProperty(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() +
vol2FileOutput
+ + ".dfs.client.hedged.read.threadpool.size", "1");
+});
+ }
+
+ @Test
+ public void testHdfsConfigOverrides() throws Exception {
+try (AccumuloClient c =
Accumulo.newClient().from(getClientProperties()).build()) {
+
+ Map siteConfig =
c.instanceOperations().getSiteConfiguration();
+ assertEquals("10485760", siteConfig
+ .get(Property.GENERAL_ARBITRARY_PROP_PREFIX.getKey() + vol1 +
".dfs.blocksize"));
+ assertEquals("104857600", siteConfig
Review Comment:
Updated test to use proposed value in e0c12c4.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3180: Enable users to provide per-volume Hadoop Filesystem overrides
dlmarion commented on code in PR #3180:
URL: https://github.com/apache/accumulo/pull/3180#discussion_r1097415253
##
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java:
##
@@ -353,6 +362,38 @@ public short getDefaultReplication(Path path) {
return getFileSystemByPath(path).getDefaultReplication(path);
}
+ /**
+ * The Hadoop Configuration object does not currently allow for duplicate
properties to be set in
+ * a single Configuration for different FileSystem URIs. Here we will look
for properties in the
+ * Accumulo configuration of the form:
+ *
+ *
+ * general.custom.volume-uri.hdfs-property
+ *
+ *
+ * We will use these properties to return a new Configuration object that
can be used with the
+ * FileSystem URI.
+ *
+ * @param conf AccumuloConfiguration object
+ * @param hadoopConf Hadoop Configuration object
+ * @param filesystemURI Volume Filesystem URI
+ * @return Hadoop Configuration with custom overrides for this FileSystem
+ */
+ private static Configuration
getVolumeManagerConfiguration(AccumuloConfiguration conf,
+ final Configuration hadoopConf, final String filesystemURI) {
+final Configuration volumeConfig = new Configuration(hadoopConf);
+final Map customProps =
+
conf.getAllPropertiesWithPrefixStripped(Property.GENERAL_ARBITRARY_PROP_PREFIX);
Review Comment:
Created new property in e0c12c4.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
