[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18106626
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java ---
@@ -18,5 +18,7 @@
public interface VolumeChooser {
+ String choose(VolumeChooserEnvironment env, String[] options);
--- End diff --
oh I just noticed that general.volume.chooser is marked experimental, so
nevermind. If it were not experimental, I like the strategy of introducing a
new prop, however it probably not needed as long as that experimental notation
makes it into the generated documentation.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18106232
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java ---
@@ -18,5 +18,7 @@
public interface VolumeChooser {
+ String choose(VolumeChooserEnvironment env, String[] options);
--- End diff --
The user facing property general.volume.chooser allows user to set classes
that implement the existing interface. That seems to put it in the public API?
One option is to deprecate that property and introduce a new property that
uses a new interface (don't base new interface on current one). If the old
prop is set, could still use the old class/interface. If both are set, then
error.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user hustjl22 commented on the pull request: https://github.com/apache/accumulo/pull/16#issuecomment-56996950 I broke this issue up into a few subtasks to break up the work into smaller sections, so I closed this pull request. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user hustjl22 closed the pull request at: https://github.com/apache/accumulo/pull/16 --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18101178
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java ---
@@ -18,5 +18,7 @@
public interface VolumeChooser {
+ String choose(VolumeChooserEnvironment env, String[] options);
--- End diff --
Well, that's just it... not being in the public API, we haven't sold it
that way yet. The argument isn't exactly "we don't *have* [to] keep it
compatible". The argument is "we have the *opportunity* to get it right before
we move it to the public API".
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18100820
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java ---
@@ -18,5 +18,7 @@
public interface VolumeChooser {
+ String choose(VolumeChooserEnvironment env, String[] options);
--- End diff --
It's not public API, but if we're selling it as "You can implement your own
policy", it should be (and I would argue we should treat it as such until that
is rectified). If there is something that fundamentally doesn't make sense and
can't be solved in other ways, let's consider that as a reason for change, but
I'd prefer not to see an argument for change based only on "we don't *have*
keep it compatible"
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18100611
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java ---
@@ -18,5 +18,7 @@
public interface VolumeChooser {
+ String choose(VolumeChooserEnvironment env, String[] options);
--- End diff --
Right, this will break the interface. This is low risk, I think, though.
The VolumeChooser isn't part of the public API (yet), is it? The alternative is
that we can pass the environment with dependency injection to an annotated
field, if it exists. If this is something that's a blocker, that's an option.
There's also a slight behavioral change to consider here (improvement, I
think) that may be relevant, if we're concerned about API. The original
VolumeChooser interface did not choose between volumes it chose between
paths. This patch changes things so that the API conforms to the user
expectations that it will be provided with volumes from which to choose, which
the framework will append any necessary paths to, if needed.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18100257
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java
---
@@ -16,14 +16,65 @@
*/
package org.apache.accumulo.server.fs;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
import java.util.Random;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
public class RandomVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(RandomVolumeChooser.class);
Random random = new Random();
-
+
+ public RandomVolumeChooser() {}
--- End diff --
Oh, I see. Yeah, I agree. The RandomVolumeChooser shouldn't change much.
The preferredVolumes stuff should be rolled into the StaticVolumeChooser, since
the static case is just a special case (only one preference). (And the
StaticVolumeChooser should probably be renamed, and moved to examples, since it
depends on custom user properties.)
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18094974
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java
---
@@ -16,14 +16,65 @@
*/
package org.apache.accumulo.server.fs;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
import java.util.Random;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
public class RandomVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(RandomVolumeChooser.class);
Random random = new Random();
-
+
+ public RandomVolumeChooser() {}
--- End diff --
Sorry, I wasn't very clear. The difference being "choose from available
volumes provided" and "choose from volumes set in a table property". They can
definitely both share the same "choose random" code, but, in the end, they do
two different things. I don't see a reason to make the `RandomVolumeChooser` do
both of these (tell me one if I'm missing it).
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18094633 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- Thanks for the details everyone. I forgot about tablets not moving across volumes (and thus the default tablet could get stuck elsewhere). No need to split out those changes or make a separate issue for them. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user hustjl22 commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18093257
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java ---
@@ -261,7 +263,8 @@ private static String decommisionedTabletDir(ZooLock
zooLock, VolumeManager vm,
throw new IllegalArgumentException("Unexpected table dir " + dir);
}
-Path newDir = new Path(vm.choose(ServerConstants.getTablesDirs()) +
"/" + dir.getParent().getName() + "/" + dir.getName());
+Path newDir = new
Path(vm.choose(Optional.of(extent.getTableId().toString()),
ServerConstants.getTablesDirs()) + "/tables/" + dir.getParent().getName()
--- End diff --
Yes ServerConstants.getTablesDirs() does give you this. To make the paths
consistant with what would come in from the table property, I took away this
part of the method. And added it back after the volume was chosen. I guess it
makes more sense to just call getBaseUris() though.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18089531 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- Could split this work into two branches. One branch for adding tables props to create table and another for this issue, and then rebase this branch on the other. I am thinking of working on ACCUMULO-1798 and ACCUMULO-3134 in two branches concurrently, with ACCUMULO-3134 branch being based on ACCUMULO-1798 branch. Was thinking this would make things easier for reviewers when I submit them for review, but I have never tried this before so uncertain of benefit. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18089107 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- > is that out of the scope of this ticket and maybe should become its own ticket? If you want to move setting table props at creation time into its own issue, that would be make sense to me. May be better to do that since proxy and shell changes will be needed. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18089006 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- > is that out of the scope of this ticket and maybe should become its own ticket? I don't think its out of scope, because either way a new method is introduced. If we are going to introduce a new method, I would advocate for using the one with better attributes. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user hustjl22 commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18088179 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- Being able to set initial table properties on table creation is nessecary for implementing this feature. I understand the advantages to introducing a NewTableConfiguration object, however is that out of the scope of this ticket and maybe should become its own ticket? --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18085325 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- The problem is that volumes are chosen once per tablet. So, if you are using the GeneralVolumeChooser provided (which is really a per-table volume chooser), there is no ability to set the chooser for this specific table until after the default one (the RandomVolumeChooser) has already made a permanent decision for the default tablet. So, one could never write a volume chooser that said "*Only* use this specific volume for this table." So, this feature is absolutely essential if you want to implement a per-table volume chooser. I suppose it could be extracted as a separate issue, which is an explicit prerequisite of this one, though. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18085196
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/GeneralVolumeChooser.java
---
@@ -0,0 +1,60 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
+import org.apache.log4j.Logger;
+
+public class GeneralVolumeChooser implements VolumeChooser {
--- End diff --
This GeneralVolumeChooser might be better named "PerTableVolumeChooser".
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18085110
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java
---
@@ -16,14 +16,65 @@
*/
package org.apache.accumulo.server.fs;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
import java.util.Random;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
public class RandomVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(RandomVolumeChooser.class);
Random random = new Random();
-
+
+ public RandomVolumeChooser() {}
--- End diff --
This comment confuses me. That's how the RandomVolumeChooser already works:
choose a random one from the given set.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on the pull request: https://github.com/apache/accumulo/pull/16#issuecomment-56919878 In general, very bold set of changes for being relatively new :). I think you're on the right path, but it definitely needs some more TLC. Feel free to reach out for advice/questions! --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073916
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
+
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073889
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
+
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073873
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
--- End diff --
Oops, timeout here too.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073847
--- Diff:
server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java
---
@@ -176,7 +178,7 @@ public long isReady(long tid, Master environment)
throws Exception {
@Override
public Repo call(long tid, Master master) throws Exception {
// Constants.DEFAULT_TABLET_LOCATION has a leading slash prepended to
it so we don't need to add one here
-tableInfo.dir =
master.getFileSystem().choose(ServerConstants.getTablesDirs()) + "/" +
tableInfo.tableId + Constants.DEFAULT_TABLET_LOCATION;
+tableInfo.dir =
master.getFileSystem().choose(Optional.of(tableInfo.tableId),
ServerConstants.getTablesDirs()) + "/tables/" + tableInfo.tableId +
Constants.DEFAULT_TABLET_LOCATION;
--- End diff --
Extra "tables" here too.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073851
--- Diff:
server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java ---
@@ -358,7 +359,7 @@ public synchronized void open(String address) throws
IOException {
log.debug("DfsLogger.open() begin");
VolumeManager fs = conf.getFileSystem();
-logPath = fs.choose(ServerConstants.getWalDirs()) + "/" + logger + "/"
+ filename;
+logPath = fs.choose(Optional. absent(),
ServerConstants.getWalDirs()) + "/wal/" + logger + "/" + filename;
--- End diff --
"wal" added to the path?
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073838
--- Diff:
server/master/src/main/java/org/apache/accumulo/master/Master.java ---
@@ -248,7 +249,7 @@ private void moveRootTabletToRootTable(IZooReaderWriter
zoo) throws Exception {
if (!zoo.exists(dirZPath)) {
Path oldPath = fs.getFullPath(FileType.TABLE, "/" + MetadataTable.ID
+ "/root_tablet");
if (fs.exists(oldPath)) {
-String newPath = fs.choose(ServerConstants.getTablesDirs()) + "/"
+ RootTable.ID;
+String newPath = fs.choose(Optional.of(RootTable.ID),
ServerConstants.getTablesDirs()) + "/tables/" + RootTable.ID;
--- End diff --
Oh, you do provide `RootTable.ID` down here. Definitely choose one or the
other. The former seems like it might be an omission?
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18073842 --- Diff: server/base/src/main/java/org/apache/accumulo/server/util/TabletOperations.java --- @@ -38,7 +40,7 @@ public static String createTabletDirectory(VolumeManager fs, String tableId, Tex String lowDirectory; UniqueNameAllocator namer = UniqueNameAllocator.getInstance(); -String volume = fs.choose(ServerConstants.getTablesDirs()); +String volume = fs.choose(Optional.of(tableId), ServerConstants.getTablesDirs()) + "/tables/"; --- End diff -- Extra "tables" here too. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18073827 --- Diff: server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java --- @@ -880,7 +882,7 @@ public static void cloneTable(Instance instance, String srcTableId, String table Key k = entry.getKey(); Mutation m = new Mutation(k.getRow()); m.putDelete(k.getColumnFamily(), k.getColumnQualifier()); - String dir = volumeManager.choose(ServerConstants.getTablesDirs()) + "/" + tableId + String dir = volumeManager.choose(Optional.of(tableId), ServerConstants.getTablesDirs()) + "/tables/" + tableId --- End diff -- "tables" here too. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073825
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java ---
@@ -79,7 +81,7 @@ public Text getLastRow() {
private static final Logger log = Logger.getLogger(FileUtil.class);
private static Path createTmpDir(AccumuloConfiguration acuConf,
VolumeManager fs) throws IOException {
-String accumuloDir = fs.choose(ServerConstants.getTemporaryDirs());
+String accumuloDir = fs.choose(Optional.absent(),
ServerConstants.getTemporaryDirs()) + "/tables/";
--- End diff --
Why the extra "tables" in the path?
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18073817 --- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java --- @@ -313,8 +313,8 @@ private static void initFileSystem(Opts opts, VolumeManager fs, UUID uuid, Path // the actual disk locations of the metadata table and tablets final Path[] metadataTableDirs = paths(ServerConstants.getMetadataTableDirs()); -String tableMetadataTabletDir = fs.choose(VolumeConfiguration.prefix(ServerConstants.getMetadataTableDirs(), TABLE_TABLETS_TABLET_DIR)); -String defaultMetadataTabletDir = fs.choose(VolumeConfiguration.prefix(ServerConstants.getMetadataTableDirs(), Constants.DEFAULT_TABLET_LOCATION)); +String tableMetadataTabletDir = fs.choose(Optional.absent(), ServerConstants.getMetadataTableDirs()) + "/tables/" + MetadataTable.ID + "/table_info"; --- End diff -- Again, a little confusing. We do have a tableId for the metadata table. Why isn't it being passed into the volume chooser? --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18073807 --- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java --- @@ -232,9 +233,8 @@ public static boolean initialize(Opts opts, String instanceNamePath, VolumeManag UUID uuid = UUID.randomUUID(); // the actual disk locations of the root table and tablets -String[] configuredTableDirs = VolumeConfiguration.prefix(VolumeConfiguration.getVolumeUris(SiteConfiguration.getInstance()), -ServerConstants.TABLE_DIR); -final Path rootTablet = new Path(fs.choose(configuredTableDirs) + "/" + RootTable.ID + RootTable.ROOT_TABLET_LOCATION); +String[] configuredTableDirs = VolumeConfiguration.getVolumeUris(SiteConfiguration.getInstance()); +final Path rootTablet = new Path(fs.choose(Optional.absent(), configuredTableDirs) + "/" + ServerConstants.TABLE_DIR+ "/" + RootTable.ID + RootTable.ROOT_TABLET_LOCATION); --- End diff -- A bit confusion to provide optional when there is a `tableId` available for the Root table. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073784
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java ---
@@ -261,7 +263,8 @@ private static String decommisionedTabletDir(ZooLock
zooLock, VolumeManager vm,
throw new IllegalArgumentException("Unexpected table dir " + dir);
}
-Path newDir = new Path(vm.choose(ServerConstants.getTablesDirs()) +
"/" + dir.getParent().getName() + "/" + dir.getName());
+Path newDir = new
Path(vm.choose(Optional.of(extent.getTableId().toString()),
ServerConstants.getTablesDirs()) + "/tables/" + dir.getParent().getName()
--- End diff --
You added an extra "tables" to the constructed `Path`. Was that
intentional? Doesn't `ServerConstants.getTablesDirs()` give this to you already?
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073730
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/StaticVolumeChooser.java
---
@@ -0,0 +1,68 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
+public class StaticVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(StaticVolumeChooser.class);
+
+ public StaticVolumeChooser() {}
+
+ @Override
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+// Get the current table's properties, and find the preferred volumes
property
+PropertyFilter filter = new AllFilter();
+Map props = new java.util.HashMap();
+env.getProperties(props, filter);
+String volume = props.get("table.custom.preferredVolumes");
+log.info("In custom chooser");
+log.info("Volumes: " + volume);
+log.info("TableID: " + env.getTableId());
+
+if (volume != null) {
+ // If preferred volume is specified, make sure that volume is an
instance volume.
+ boolean usePreferred = false;
+ for (String s : options) {
+if (volume.equals(s)) {
+ usePreferred = true;
+ break;
+}
+ }
+
+ // If the preferred volume is an instance volume return that volume.
+ if (usePreferred)
+return volume;
+ // Otherwise warn the user that it is not an instance volume and
that the chooser will default to randomly choosing from the instance volumes.
+ else
+log.warn("Preferred volume, " + volume + ", is not an instance
volume. Defaulting to randomly choosing from instance volumes");
+}
+// If the preferred volumes property is not specified, warn the user,
then use a RandomVolumeChoser to choose randomly from the given list of volumes
+log.warn("No preferred volume specified. Defaulting to randomly
choosing from instance volumes");
+return new RandomVolumeChooser().choose(options);
--- End diff --
Lots of object creation and garbage collection. Couldn't this extend
RandomVolumeChooser and call `super.choose(options)` when it can't find a
static volume?
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073736
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/StaticVolumeChooser.java
---
@@ -0,0 +1,68 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
+public class StaticVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(StaticVolumeChooser.class);
+
+ public StaticVolumeChooser() {}
+
+ @Override
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+// Get the current table's properties, and find the preferred volumes
property
+PropertyFilter filter = new AllFilter();
+Map props = new java.util.HashMap();
+env.getProperties(props, filter);
+String volume = props.get("table.custom.preferredVolumes");
+log.info("In custom chooser");
+log.info("Volumes: " + volume);
+log.info("TableID: " + env.getTableId());
+
+if (volume != null) {
+ // If preferred volume is specified, make sure that volume is an
instance volume.
+ boolean usePreferred = false;
+ for (String s : options) {
+if (volume.equals(s)) {
+ usePreferred = true;
+ break;
+}
+ }
+
+ // If the preferred volume is an instance volume return that volume.
+ if (usePreferred)
+return volume;
+ // Otherwise warn the user that it is not an instance volume and
that the chooser will default to randomly choosing from the instance volumes.
+ else
+log.warn("Preferred volume, " + volume + ", is not an instance
volume. Defaulting to randomly choosing from instance volumes");
+}
+// If the preferred volumes property is not specified, warn the user,
then use a RandomVolumeChoser to choose randomly from the given list of volumes
+log.warn("No preferred volume specified. Defaulting to randomly
choosing from instance volumes");
+return new RandomVolumeChooser().choose(options);
+ }
+
+ @Override
+ public String choose(String[] options) {
+// If table is not specified, use a RandomVolumeChoser to choose
randomly from the given options
+return new RandomVolumeChooser().choose(options);
--- End diff --
Same about object creation and extending RandomVolumeChooser.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073713
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/StaticVolumeChooser.java
---
@@ -0,0 +1,68 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
+public class StaticVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(StaticVolumeChooser.class);
+
+ public StaticVolumeChooser() {}
+
+ @Override
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+// Get the current table's properties, and find the preferred volumes
property
+PropertyFilter filter = new AllFilter();
+Map props = new java.util.HashMap();
+env.getProperties(props, filter);
+String volume = props.get("table.custom.preferredVolumes");
+log.info("In custom chooser");
--- End diff --
Fine for debugging, but these need to be removed or pulled back to trace
logging.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073703
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/StaticVolumeChooser.java
---
@@ -0,0 +1,68 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
+public class StaticVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(StaticVolumeChooser.class);
+
+ public StaticVolumeChooser() {}
+
+ @Override
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+// Get the current table's properties, and find the preferred volumes
property
+PropertyFilter filter = new AllFilter();
+Map props = new java.util.HashMap();
+env.getProperties(props, filter);
+String volume = props.get("table.custom.preferredVolumes");
--- End diff --
Again, no hard-coded, inline'd configuration keys.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073696
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/StaticVolumeChooser.java
---
@@ -0,0 +1,68 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
+public class StaticVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(StaticVolumeChooser.class);
+
+ public StaticVolumeChooser() {}
--- End diff --
Need some unit tests for `StaticVolumeChooser`
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073661
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/GeneralVolumeChooser.java
---
@@ -0,0 +1,60 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
+import org.apache.log4j.Logger;
+
+public class GeneralVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(GeneralVolumeChooser.class);
+
+ public GeneralVolumeChooser() {}
+
+ @Override
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+String clazzName = new String();
+try {
+ // Get the current table's properties, and find the chooser property
+ PropertyFilter filter = new AllFilter();
+ Map props = new java.util.HashMap();
+ env.getProperties(props, filter);
+
+ clazzName = props.get("table.custom.chooser");
--- End diff --
Again, should be some public constant, likely in Property and something
that ties it to the `GeneralVolumeChooser` class
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073654
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/GeneralVolumeChooser.java
---
@@ -0,0 +1,60 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
+import org.apache.log4j.Logger;
+
+public class GeneralVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(GeneralVolumeChooser.class);
+
+ public GeneralVolumeChooser() {}
+
+ @Override
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+String clazzName = new String();
+try {
+ // Get the current table's properties, and find the chooser property
+ PropertyFilter filter = new AllFilter();
+ Map props = new java.util.HashMap();
+ env.getProperties(props, filter);
+
+ clazzName = props.get("table.custom.chooser");
+ log.info("TableID: " + env.getTableId() + " ClassName: " +
clazzName);
+
+ // Load the correct chooser and create an instance of it
+ Class clazz =
AccumuloVFSClassLoader.loadClass(clazzName, VolumeChooser.class);
--- End diff --
This is going to be rather expensive to load the class and instantiate it
every single time we choose a volume for a file WRT how quick it should be.
Make sure the implementations are thread-safe and aren't sharing state and you
can keep a cache of implementations around. (although this has issues when
considering dynamic classloading)
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073602
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/GeneralVolumeChooser.java
---
@@ -0,0 +1,60 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
+import org.apache.log4j.Logger;
+
+public class GeneralVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(GeneralVolumeChooser.class);
+
+ public GeneralVolumeChooser() {}
+
+ @Override
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+String clazzName = new String();
+try {
+ // Get the current table's properties, and find the chooser property
+ PropertyFilter filter = new AllFilter();
+ Map props = new java.util.HashMap();
+ env.getProperties(props, filter);
+
+ clazzName = props.get("table.custom.chooser");
+ log.info("TableID: " + env.getTableId() + " ClassName: " +
clazzName);
+
+ // Load the correct chooser and create an instance of it
+ Class clazz =
AccumuloVFSClassLoader.loadClass(clazzName, VolumeChooser.class);
+ VolumeChooser instance = clazz.newInstance();
+ // Choose the volume based using the created chooser
+ return instance.choose(env, options);
+} catch (Exception e) {
+ // If an exception occurs, first write a warning and then use a
default RandomVolumeChooser to choose from the given options
+ log.warn(e, e);
+ return new RandomVolumeChooser().choose(options);
--- End diff --
IMO, it's much better to fail hard and fast if the configuration is busted
(or the classpath is missing necessary classes). It may require some better,
graceful error handling in the caller, but we can help figure that out.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073577
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java
---
@@ -16,14 +16,65 @@
*/
package org.apache.accumulo.server.fs;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
import java.util.Random;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
public class RandomVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(RandomVolumeChooser.class);
Random random = new Random();
-
+
+ public RandomVolumeChooser() {}
+
@Override
- public String choose(String[] options) {
-return options[random.nextInt(options.length)];
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+// Get the current table's properties, and find the preferred volumes
property
+PropertyFilter filter = new AllFilter();
+Map props = new java.util.HashMap();
+ArrayList prefVol = new ArrayList();
+env.getProperties(props, filter);
+String volumes = props.get("table.custom.preferredVolumes");
--- End diff --
Thinking some more, if this does become its own standalone implementation,
the value should also be changed so that it reflects usage by this
implementation. e.g if you named the new class `FooVolumeChooser` the property
could be something like `table.foo.volume.candidates` or similar (hopefully
that's clear)
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073546
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java
---
@@ -16,14 +16,65 @@
*/
package org.apache.accumulo.server.fs;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
import java.util.Random;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
public class RandomVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(RandomVolumeChooser.class);
Random random = new Random();
-
+
+ public RandomVolumeChooser() {}
--- End diff --
Actually, this is really confusing because when the table property is set,
this isn't a "random volume" chooser at all -- it's a "choose a volume from
this set" chooser. It would make more sense to me for this code to live in its
own volume chooser implementation rather than push itself into the existing
RandomVolumeChooser.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073509
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java
---
@@ -16,14 +16,65 @@
*/
package org.apache.accumulo.server.fs;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
import java.util.Random;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
public class RandomVolumeChooser implements VolumeChooser {
+ private static final Logger log =
Logger.getLogger(RandomVolumeChooser.class);
Random random = new Random();
-
+
+ public RandomVolumeChooser() {}
+
@Override
- public String choose(String[] options) {
-return options[random.nextInt(options.length)];
+ public String choose(VolumeChooserEnvironment env, String[] options) {
+// Get the current table's properties, and find the preferred volumes
property
+PropertyFilter filter = new AllFilter();
+Map props = new java.util.HashMap();
+ArrayList prefVol = new ArrayList();
+env.getProperties(props, filter);
+String volumes = props.get("table.custom.preferredVolumes");
--- End diff --
Should be an element in Property, not a hard-coded, inline'd string.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073494
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/StaticVolumeChooser.java
---
@@ -0,0 +1,68 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.log4j.Logger;
+
+public class StaticVolumeChooser implements VolumeChooser {
--- End diff --
If this is being provided for users to leverage, it needs unit tests.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073482
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
+
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073480
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
+
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073475
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
+
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073474
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
+
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073477
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
+
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r18073417
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java ---
@@ -18,5 +18,7 @@
public interface VolumeChooser {
+ String choose(VolumeChooserEnvironment env, String[] options);
--- End diff --
Plus, having duplicative methods like this is confusing for an
implementation (when does choose1() get called and when does choose2() get
called, why, etc). Maybe create some utility/helper method for implementations
to get the current VolumeChooserEnvironment?
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18073371 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- That's a use case, but it isn't a necessity. Why can't the existing table property configuration methods be used? --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18073355 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- The extra constructor is needed so that the per-table volume chooser can be specified before the default tablet gets created. Currently, the RPC supports setting initial properties, but we didn't expose that in the API. We just used it for initial iterators. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r18073259 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- Why are we adding more constructors at all? It seems unrelated to the nature of the title. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r17989615 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- This new functionality probably needs to be exposed in the shell and proxy. This occurred to me when I was thinking about writing user documentation for this. The documentation would probably use the shell. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r17989515
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/GeneralVolumeChooser.java
---
@@ -0,0 +1,60 @@
+/*
+ * 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
+ *
+ * http://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.server.fs;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.conf.AccumuloConfiguration.AllFilter;
+import org.apache.accumulo.core.conf.AccumuloConfiguration.PropertyFilter;
+import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
+import org.apache.log4j.Logger;
+
+public class GeneralVolumeChooser implements VolumeChooser {
--- End diff --
User facing documentation is needed somewhere to inform users how they can
put these differently volume choosers and the custom props together. Should
also tell explain why they should use create table w/ props for this.
Could do that as javadoc on these volume choosers and/or in the user manual.
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r17988361
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r17987986
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
---
@@ -560,8 +565,22 @@ public ContentSummary getContentSummary(Path dir)
throws IOException {
}
@Override
- public String choose(String[] options) {
-return chooser.choose(options);
+ public String choose(Optional tableId, String[] options) {
+// If the tableId is present, use that to create a
VolumeChooserEnvironment variable
+if (tableId.isPresent()) {
+ // Get the current instance and from that the
ServerConfigurationFactory and in turn the tableId
+ Instance instance = HdfsZooInstance.getInstance();
+ ServerConfigurationFactory serverConf = new
ServerConfigurationFactory(instance);
+ TableConfiguration tableConf =
serverConf.getTableConfiguration(tableId.get());
+
+ // Create the environment and then choose the volume using the
currently defined chooser
+ VolumeChooserEnvironment env = new
VolumeChooserEnvironment(tableConf);
+ return chooser.choose(env, options);
+} else {
+ // If the tableId is missing, then just choose using the current
chooser, without using the per table properties
+ log.info("TABLE ID MISSING");
--- End diff --
this is not a very useful log message, and I assume it will occur regularly
in the case when a volume is chosen for walogs... maybe drop it
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r17987756
--- Diff:
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java ---
@@ -18,5 +18,7 @@
public interface VolumeChooser {
+ String choose(VolumeChooserEnvironment env, String[] options);
--- End diff --
ugh, this will break any current code that has implemented the
interface Java 8 has a solution for this, but not 7
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r17987487
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r17987372
--- Diff: test/src/test/java/org/apache/accumulo/test/VolumeChooserIT.java
---
@@ -0,0 +1,670 @@
+/*
+ * 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
+ *
+ * http://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.Assert.*;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.TimeType;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.metadata.MetadataTable;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.functional.ConfigurableMacIT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class VolumeChooserIT extends ConfigurableMacIT {
+
+ private static final Text EMPTY = new Text();
+ private static final Value EMPTY_VALUE = new Value(new byte[] {});
+ private File volDirBase;
+ private Path v1, v2, v3, v4;
+
+ @Override
+ public void configure(MiniAccumuloConfigImpl cfg, Configuration
hadoopCoreSite) {
+// Get 2 tablet servers
+cfg.setNumTservers(2);
+
+// Set the general volume chooser to the GeneralVolumeChooser so that
different choosers can be specified
+Map siteConfig = new HashMap();
+siteConfig.put(Property.GENERAL_VOLUME_CHOOSER.getKey(),
org.apache.accumulo.server.fs.GeneralVolumeChooser.class.getName());
+cfg.setSiteConfig(siteConfig);
+
+// Set up 4 different volume paths
+File baseDir = cfg.getDir();
+volDirBase = new File(baseDir, "volumes");
+File v1f = new File(volDirBase, "v1");
+File v2f = new File(volDirBase, "v2");
+File v3f = new File(volDirBase, "v3");
+File v4f = new File(volDirBase, "v4");
+v1f.mkdir();
+v2f.mkdir();
+v4f.mkdir();
+v1 = new Path("file://" + v1f.getAbsolutePath());
+v2 = new Path("file://" + v2f.getAbsolutePath());
+v3 = new Path("file://" + v3f.getAbsolutePath());
+v4 = new Path("file://" + v4f.getAbsolutePath());
+
+// Only add volumes 1, 2, and 4 to the list of instance volumes to
have one volume that isn't in the options list when they are choosing
+cfg.setProperty(Property.INSTANCE_VOLUMES, v1.toString() + "," +
v2.toString() + "," + v4.toString());
+
+// use raw local file system so walogs sync and flush will work
+hadoopCoreSite.set("fs.file.impl", RawLocalFileSystem.class.getName());
+
+super.configure(cfg, hadoopCoreSite);
+
+ }
+
+ // Test that uses two tables with 10 split points each. They each use
the StaticVolumeChooser to choose volumes.
+ @Test(timeout = 60 * 1000)
+ public void twoTablesStaticVolumeChooser() throws Exception {
+log.info("Starting StaticVolumeChooser");
+
+// Create and populate initial properties map for creating table 1
+Map properties = new HashMap();
+String propertyName = "table.custom.chooser";
+String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/16#discussion_r17987075
--- Diff:
core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
---
@@ -103,6 +103,25 @@
void create(String tableName, boolean versioningIter, TimeType timeType)
throws AccumuloException, AccumuloSecurityException, TableExistsException;
/**
+ * @param tableName
+ * the name of the table
+ * @param limitVersion
+ * Enables/disables the versioning iterator, which will limit
the number of Key versions kept.
+ * @param timeType
+ * specifies logical or real-time based time recording for
entries in the table
+ * @param properties
+ * initial table properties the user wants
+ * @throws AccumuloException
+ * if a general error occurs
+ * @throws AccumuloSecurityException
+ * if the user does not have permission
+ * @throws TableExistsException
+ * if the table already exists
+ */
+ void create(String tableName, boolean limitVersion, TimeType timeType,
Map properties) throws AccumuloException,
AccumuloSecurityException,
--- End diff --
Instead of introducing yet another create table method, we could create a
NewTableConfiguration class and pass that.
```java
public class NewTableConfiguration {
public NewTableConfiguration setTimeType(TimeType tt){...}
public NewTableConfiguration setLimitVersions(boolean lv){...}
public NewTableConfiguration setProperties(Map
properties){...}
}
```
And add a method like the following, and possibly deprecated existing
create table methods (except for create(String) )
```java
void create(String tableName, NewTableConfig ntc)
```
---
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.
---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r17931845 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants + * @throws AccumuloException + * if a general error occurs + * @throws AccumuloSecurityException + * if the user does not have permission + * @throws TableExistsException + * if the table already exists + */ + void create(String tableName, boolean limitVersion, TimeType timeType, Map properties) throws AccumuloException, AccumuloSecurityException, --- End diff -- This added method needs additional tests to verify correct behavior. --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
Github user ctubbsii commented on a diff in the pull request: https://github.com/apache/accumulo/pull/16#discussion_r17931715 --- Diff: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java --- @@ -103,6 +103,25 @@ void create(String tableName, boolean versioningIter, TimeType timeType) throws AccumuloException, AccumuloSecurityException, TableExistsException; /** + * @param tableName + * the name of the table + * @param limitVersion + * Enables/disables the versioning iterator, which will limit the number of Key versions kept. + * @param timeType + * specifies logical or real-time based time recording for entries in the table + * @param properties + * initial table properties the user wants --- End diff -- This javadoc should explain how the initial table properties are are merged with the properties generated from the other arguments. (such as the limitVersion argument) --- 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. ---
[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...
GitHub user hustjl22 opened a pull request: https://github.com/apache/accumulo/pull/16 ACCUMULO-3089: Create volume chooser from table properties You can merge this pull request into a Git repository by running: $ git pull https://github.com/hustjl22/accumulo ACCUMULO-3089 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/accumulo/pull/16.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #16 commit 202252344a0ba11eb8121dcbbb37604a91852d35 Author: Jenna Huston Date: 2014-09-23T13:58:24Z ACCUMULO-3089: Create volume chooser from table properties --- 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. ---
