You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by hustjl22 <gi...@git.apache.org> on 2014/09/23 19:45:26 UTC

[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 <je...@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    --- End diff --
    
    Nice test in terms of functionality covered!  If you could condense some of this repeated code for reading and writing data into functions, it would make the test easier to read.   


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<Master> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by hustjl22 <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +      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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +      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<? extends VolumeChooser> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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.<String>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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +      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<? extends VolumeChooser> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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.<String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by hustjl22 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by hustjl22 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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.<String>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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. They each use the RandomVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    --- End diff --
    
    Move test timeout to defaultTimeoutSeconds (Override-able from AbstractMacIT) so it will use the timeout.factor scaling for slower hardware.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. They each use the RandomVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void twoTablesRandomVolumeChooser() throws Exception {
    +    log.info("Starting RandomVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString() + "," + v2.toString() + "," + v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV1 || inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. The first uses the RandomVolumeChooser and the second uses the
    +  // StaticVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void twoTablesDiffChoosers() throws Exception {
    +    log.info("Starting twoTablesDiffChoosers");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString() + "," + v2.toString() + "," + v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +
    +    // Add 10 splits to the table
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV1 || inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current chooser preferredVolumes property for table 2
    +    properties.remove("table.custom.chooser");
    +    propertyName = "table.custom.chooser";
    +    volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It uses the StaticVolumeChooser, but no preferred volume is specified. This means that the volume
    +  // is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void missingVolumeStaticVolumeChooser() throws Exception {
    +    log.info("Starting missingVolumeStaticVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      boolean inV4 = entry.getKey().getColumnQualifier().toString().contains(v4.toString());
    +      assertTrue(inV1 || inV2 || inV4);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It uses the RandomVolumeChooser, but no preferred volume is specified. This means that the volume
    +  // is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void missingVolumeRandomVolumeChooser() throws Exception {
    +    log.info("Starting missingVolumeRandomVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      boolean inV4 = entry.getKey().getColumnQualifier().toString().contains(v4.toString());
    +      assertTrue(inV1 || inV2 || inV4);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It uses the StaticVolumeChooser, but preferred volume is not an instance volume. This means that the
    +  // volume is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    --- End diff --
    
    Move test timeout to defaultTimeoutSeconds (Override-able from AbstractMacIT) so it will use the timeout.factor scaling for slower hardware.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. They each use the RandomVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void twoTablesRandomVolumeChooser() throws Exception {
    +    log.info("Starting RandomVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString() + "," + v2.toString() + "," + v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV1 || inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. The first uses the RandomVolumeChooser and the second uses the
    +  // StaticVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void twoTablesDiffChoosers() throws Exception {
    +    log.info("Starting twoTablesDiffChoosers");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString() + "," + v2.toString() + "," + v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +
    +    // Add 10 splits to the table
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV1 || inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current chooser preferredVolumes property for table 2
    +    properties.remove("table.custom.chooser");
    +    propertyName = "table.custom.chooser";
    +    volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It uses the StaticVolumeChooser, but no preferred volume is specified. This means that the volume
    +  // is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    --- End diff --
    
    Move test timeout to defaultTimeoutSeconds (Override-able from AbstractMacIT) so it will use the timeout.factor scaling for slower hardware.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    --- End diff --
    
    You can use `MetadataSchema` here to make more readable code instead of constructing the ranges/scanner by hand


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. They each use the RandomVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void twoTablesRandomVolumeChooser() throws Exception {
    +    log.info("Starting RandomVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString() + "," + v2.toString() + "," + v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV1 || inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. The first uses the RandomVolumeChooser and the second uses the
    +  // StaticVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    --- End diff --
    
    Move test timeout to defaultTimeoutSeconds (Override-able from AbstractMacIT) so it will use the timeout.factor scaling for slower hardware.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String,String> 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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. They each use the RandomVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void twoTablesRandomVolumeChooser() throws Exception {
    +    log.info("Starting RandomVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString() + "," + v2.toString() + "," + v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV1 || inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    --- End diff --
    
    It's nice to include a message as to why the condition failed. Makes it much easier to debug a failing test down the road.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +    ArrayList<String> prefVol = new ArrayList<String>();
    +    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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.<String>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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    --- End diff --
    
    Could do the following, not sure if method names to get name is correct
    
    ```java
      String volume = StaticVolumeChooser.class.getName()
    ```


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +    ArrayList<String> prefVol = new ArrayList<String>();
    +    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by keith-turner <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    --- End diff --
    
    would you be able to replace these bits of code that verfiy volumes used w/ a function?  Seems there is similar code repeated.   


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> siteConfig = new HashMap<String,String>();
    +    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<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v2.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. They each use the RandomVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void twoTablesRandomVolumeChooser() throws Exception {
    +    log.info("Starting RandomVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString() + "," + v2.toString() + "," + v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV1 || inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current preferredVolumes property for table 2
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses two tables with 10 split points each. The first uses the RandomVolumeChooser and the second uses the
    +  // StaticVolumeChooser to choose volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void twoTablesDiffChoosers() throws Exception {
    +    log.info("Starting twoTablesDiffChoosers");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString() + "," + v2.toString() + "," + v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +
    +    // Add 10 splits to the table
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      assertTrue(inV1 || inV2);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +
    +    // Replace the current chooser preferredVolumes property for table 2
    +    properties.remove("table.custom.chooser");
    +    propertyName = "table.custom.chooser";
    +    volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v1.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create table 2 with the new properties
    +    String tableName2 = getUniqueNames(2)[1];
    +    connector.tableOperations().create(tableName2, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName2, partitions);
    +    // Write some data to the table
    +    bw = connector.createBatchWriter(tableName2, new BatchWriterConfig());
    +    rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName2, null, null, true);
    +    scanner = connector.createScanner(tableName2, Authorizations.EMPTY);
    +    i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +    // Verify the new files are written to the different volumes
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("2", "2<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      assertTrue(inV1);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It uses the StaticVolumeChooser, but no preferred volume is specified. This means that the volume
    +  // is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void missingVolumeStaticVolumeChooser() throws Exception {
    +    log.info("Starting missingVolumeStaticVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      boolean inV4 = entry.getKey().getColumnQualifier().toString().contains(v4.toString());
    +      assertTrue(inV1 || inV2 || inV4);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It uses the RandomVolumeChooser, but no preferred volume is specified. This means that the volume
    +  // is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void missingVolumeRandomVolumeChooser() throws Exception {
    +    log.info("Starting missingVolumeRandomVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      boolean inV4 = entry.getKey().getColumnQualifier().toString().contains(v4.toString());
    +      assertTrue(inV1 || inV2 || inV4);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It uses the StaticVolumeChooser, but preferred volume is not an instance volume. This means that the
    +  // volume is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void notInstanceStaticVolumeChooser() throws Exception {
    +    log.info("Starting notInstanceStaticVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.StaticVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      boolean inV4 = entry.getKey().getColumnQualifier().toString().contains(v4.toString());
    +      assertTrue(inV1 || inV2 || inV4);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It uses the RandomVolumeChooser, but preferred volume is not an instance volume. This means that the
    +  // volume is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    +  public void notInstanceRandomVolumeChooser() throws Exception {
    +    log.info("Starting notInstanceRandomVolumeChooser");
    +
    +    // Create and populate initial properties map for creating table 1
    +    Map<String,String> properties = new HashMap<String,String>();
    +    String propertyName = "table.custom.chooser";
    +    String volume = "org.apache.accumulo.server.fs.RandomVolumeChooser";
    +    properties.put(propertyName, volume);
    +
    +    properties.remove("table.custom.preferredVolumes");
    +    propertyName = "table.custom.preferredVolumes";
    +    volume = v3.toString();
    +    properties.put(propertyName, volume);
    +
    +    // Create a table with the initial properties
    +    Connector connector = getConnector();
    +    String tableName = getUniqueNames(2)[0];
    +    connector.tableOperations().create(tableName, true, TimeType.MILLIS, properties);
    +
    +    // Add 10 splits to the table
    +    SortedSet<Text> partitions = new TreeSet<Text>();
    +    for (String s : "b,e,g,j,l,o,q,t,v,y".split(","))
    +      partitions.add(new Text(s));
    +    connector.tableOperations().addSplits(tableName, partitions);
    +    // Write some data to the table
    +    BatchWriter bw = connector.createBatchWriter(tableName, new BatchWriterConfig());
    +    String[] rows = "a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z".split(",");
    +    for (String s : rows) {
    +      Mutation m = new Mutation(new Text(s));
    +      m.put(EMPTY, EMPTY, EMPTY_VALUE);
    +      bw.addMutation(m);
    +    }
    +    bw.close();
    +    // Write the data to disk, read it back
    +    connector.tableOperations().flush(tableName, null, null, true);
    +    Scanner scanner = connector.createScanner(tableName, Authorizations.EMPTY);
    +    int i = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      assertEquals(rows[i++], entry.getKey().getRow().toString());
    +    }
    +
    +    // Verify the new files are written to the Volumes specified
    +    scanner = connector.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
    +    scanner.setRange(new Range("1", "1<"));
    +    scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
    +    int fileCount = 0;
    +    for (Entry<Key,Value> entry : scanner) {
    +      boolean inV1 = entry.getKey().getColumnQualifier().toString().contains(v1.toString());
    +      boolean inV2 = entry.getKey().getColumnQualifier().toString().contains(v2.toString());
    +      boolean inV4 = entry.getKey().getColumnQualifier().toString().contains(v4.toString());
    +      assertTrue(inV1 || inV2 || inV4);
    +      fileCount++;
    +    }
    +    assertEquals(11, fileCount);
    +  }
    +
    +  // Test that uses one table with 10 split points each. It does not specify a specific chooser, so the volume is chosen randomly from all instance volumes.
    +  @Test(timeout = 60 * 1000)
    --- End diff --
    
    Move test timeout to defaultTimeoutSeconds (Override-able from AbstractMacIT) so it will use the timeout.factor scaling for slower hardware.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by hustjl22 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> props = new java.util.HashMap<String,String>();
    +    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by joshelser <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] accumulo pull request: ACCUMULO-3089: Create volume chooser from t...

Posted by ctubbsii <gi...@git.apache.org>.
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<String,String> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---