You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2018/11/22 00:00:54 UTC

[GitHub] milleruntime closed pull request #777: Remove hard coded values from BatchWriterConfig

milleruntime closed pull request #777: Remove hard coded values from BatchWriterConfig
URL: https://github.com/apache/accumulo/pull/777
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java b/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java
index b27732f116..cd400b1de9 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java
@@ -17,6 +17,11 @@
 package org.apache.accumulo.core.client;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.accumulo.core.conf.ClientProperty.BATCH_WRITER_DURABILITY;
+import static org.apache.accumulo.core.conf.ClientProperty.BATCH_WRITER_MAX_LATENCY_SEC;
+import static org.apache.accumulo.core.conf.ClientProperty.BATCH_WRITER_MAX_MEMORY_BYTES;
+import static org.apache.accumulo.core.conf.ClientProperty.BATCH_WRITER_MAX_TIMEOUT_SEC;
+import static org.apache.accumulo.core.conf.ClientProperty.BATCH_WRITER_MAX_WRITE_THREADS;
 
 import java.io.DataInput;
 import java.io.DataOutput;
@@ -26,6 +31,7 @@
 import java.util.concurrent.TimeUnit;
 
 import org.apache.accumulo.core.clientImpl.DurabilityImpl;
+import org.apache.accumulo.core.conf.ClientProperty;
 import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.apache.hadoop.io.Writable;
 import org.apache.hadoop.util.StringUtils;
@@ -37,21 +43,32 @@
  */
 public class BatchWriterConfig implements Writable {
 
-  private static final Long DEFAULT_MAX_MEMORY = 50 * 1024 * 1024L;
+  private static final Long DEFAULT_MAX_MEMORY = Long
+      .parseLong(BATCH_WRITER_MAX_MEMORY_BYTES.getDefaultValue());
   private Long maxMemory = null;
 
-  private static final Long DEFAULT_MAX_LATENCY = 2 * 60 * 1000L;
+  private static final Long DEFAULT_MAX_LATENCY = TimeUnit.MILLISECONDS
+      .convert(Long.parseLong(BATCH_WRITER_MAX_LATENCY_SEC.getDefaultValue()), TimeUnit.SECONDS);
   private Long maxLatency = null;
 
-  private static final Long DEFAULT_TIMEOUT = Long.MAX_VALUE;
+  private static final Long DEFAULT_TIMEOUT = getDefaultTimeout();
   private Long timeout = null;
 
-  private static final Integer DEFAULT_MAX_WRITE_THREADS = 3;
+  private static final Integer DEFAULT_MAX_WRITE_THREADS = Integer
+      .parseInt(BATCH_WRITER_MAX_WRITE_THREADS.getDefaultValue());
   private Integer maxWriteThreads = null;
 
   private Durability durability = Durability.DEFAULT;
   private boolean isDurabilitySet = false;
 
+  private static Long getDefaultTimeout() {
+    Long def = Long.parseLong(BATCH_WRITER_MAX_TIMEOUT_SEC.getDefaultValue());
+    if (def.equals(0L))
+      return Long.MAX_VALUE;
+    else
+      return TimeUnit.MILLISECONDS.convert(def, TimeUnit.SECONDS);
+  }
+
   /**
    * Sets the maximum memory to batch before writing. The smaller this value, the more frequently
    * the {@link BatchWriter} will write.<br>
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ClientProperty.java b/core/src/main/java/org/apache/accumulo/core/conf/ClientProperty.java
index 8801507828..a1ef2c5f9b 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/ClientProperty.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/ClientProperty.java
@@ -56,13 +56,14 @@
   BATCH_WRITER_MAX_LATENCY_SEC("batch.writer.max.latency.sec", "120",
       "Max amount of time (in seconds) to hold data in memory before flushing it"),
   BATCH_WRITER_MAX_TIMEOUT_SEC("batch.writer.max.timeout.sec", "0",
-      "Max amount" + " of time (in seconds) an unresponsive server will be re-tried. An"
+      "Max amount of time (in seconds) an unresponsive server will be re-tried. An"
           + " exception is thrown when this timeout is exceeded. Set to zero for no timeout."),
   BATCH_WRITER_MAX_WRITE_THREADS("batch.writer.max.write.threads", "3",
       "Maximum number of threads to use for writing data to tablet servers."),
-  BATCH_WRITER_DURABILITY("batch.writer.durability", "default",
-      "Change the" + " durability for the BatchWriter session. To use the table's durability"
-          + " setting. use \"default\" which is the table's durability setting."),
+  BATCH_WRITER_DURABILITY("batch.writer.durability", "default", Property.TABLE_DURABILITY
+      .getDescription() + " Setting this property will "
+      + "change the durability for the BatchWriter session. A value of \"default\" will use the "
+      + "table's durability setting. "),
 
   // Scanner
   SCANNER_BATCH_SIZE("scanner.batch.size", "1000",
diff --git a/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java b/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java
index daf40f3b61..e5c005115d 100644
--- a/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java
@@ -25,8 +25,10 @@
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.accumulo.core.conf.ClientProperty;
 import org.junit.Test;
 
 public class BatchWriterConfigTest {
@@ -235,4 +237,12 @@ private void checkBytes(BatchWriterConfig bwConfig, byte[] bytes) throws IOExcep
     assertEquals(bwConfig.getMaxWriteThreads(), createdConfig.getMaxWriteThreads());
   }
 
+  @Test
+  public void countClientProps() {
+    // count the number in case one gets added to in one place but not the other
+    ClientProperty[] bwProps = Arrays.stream(ClientProperty.values())
+        .filter(c -> c.name().startsWith("BATCH_WRITER")).toArray(ClientProperty[]::new);
+    assertEquals(5, bwProps.length);
+  }
+
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services