Author: ctubbsii
Date: Thu Jan 24 22:51:06 2013
New Revision: 1438248
URL: http://svn.apache.org/viewvc?rev=1438248&view=rev
Log:
ACCUMULO-984 Prevent unexpected behavior when using a really small timeout
values, that get truncated to zero on conversion internally.
ACCUMULO-955 Removed the minimum value for maxMemory, and accept any
non-negative value.
Modified:
accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java
accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java
Modified:
accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java
URL:
http://svn.apache.org/viewvc/accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java?rev=1438248&r1=1438247&r2=1438248&view=diff
==============================================================================
---
accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java
(original)
+++
accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/BatchWriterConfig.java
Thu Jan 24 22:51:06 2013
@@ -45,27 +45,45 @@ public class BatchWriterConfig implement
private Integer maxWriteThreads = null;
/**
+ * Sets the maximum memory to batch before writing. The smaller this value,
the more frequently the {@link BatchWriter} will write.<br />
+ * If set to a value smaller than a single mutation, then it will {@link
BatchWriter#flush()} after each added mutation. Must be non-negative.
+ *
+ * <p>
+ * <b>Default:</b> 50M
*
* @param maxMemory
- * size in bytes of the maximum memory to batch before writing.
Minimum 1K. Defaults to 50M.
+ * max size in bytes
+ * @throws IllegalArgumentException
+ * if {@code maxMemory} is less than 0
+ * @return {@code this} to allow chaining of set methods
*/
-
public BatchWriterConfig setMaxMemory(long maxMemory) {
- if (maxMemory < 1024)
- throw new IllegalArgumentException("Max memory is too low at " +
maxMemory + ". Minimum 1K.");
+ if (maxMemory < 0)
+ throw new IllegalArgumentException("Max memory must be non-negative.");
this.maxMemory = maxMemory;
return this;
}
/**
+ * Sets the maximum amount of time to hold the data in memory before
flushing it to servers.<br />
+ * For no maximum, set to zero, or {@link Long#MAX_VALUE} with {@link
TimeUnit#MILLISECONDS}.
+ *
+ * <p>
+ * {@link TimeUnit#MICROSECONDS} or {@link TimeUnit#NANOSECONDS} will be
truncated to the nearest {@link TimeUnit#MILLISECONDS}.<br />
+ * If this truncation would result in making the value zero when it was
specified as non-zero, then a minimum value of one {@link
TimeUnit#MILLISECONDS} will
+ * be used.
+ *
+ * <p>
+ * <b>Default:</b> 120 seconds
+ *
* @param maxLatency
- * The maximum amount of time to hold data in memory before
flushing it to servers. For no max set to zero or Long.MAX_VALUE with
TimeUnit.MILLIS.
- * Defaults to 120 seconds.
+ * the maximum latency, in the unit specified by the value of
{@code timeUnit}
* @param timeUnit
- * Determines how maxLatency will be interpreted.
- * @return this to allow chaining of set methods
+ * determines how {@code maxLatency} will be interpreted
+ * @throws IllegalArgumentException
+ * if {@code maxLatency} is less than 0
+ * @return {@code this} to allow chaining of set methods
*/
-
public BatchWriterConfig setMaxLatency(long maxLatency, TimeUnit timeUnit) {
if (maxLatency < 0)
throw new IllegalArgumentException("Negative max latency not allowed " +
maxLatency);
@@ -73,19 +91,31 @@ public class BatchWriterConfig implement
if (maxLatency == 0)
this.maxLatency = Long.MAX_VALUE;
else
- this.maxLatency = timeUnit.toMillis(maxLatency);
+ // make small, positive values that truncate to 0 when converted use the
minimum millis instead
+ this.maxLatency = Math.max(1, timeUnit.toMillis(maxLatency));
return this;
}
/**
+ * Sets the maximum amount of time an unresponsive server will be re-tried.
When this timeout is exceeded, the {@link BatchWriter} should throw an
exception.<br />
+ * For no timeout, set to zero, or {@link Long#MAX_VALUE} with {@link
TimeUnit#MILLISECONDS}.
+ *
+ * <p>
+ * {@link TimeUnit#MICROSECONDS} or {@link TimeUnit#NANOSECONDS} will be
truncated to the nearest {@link TimeUnit#MILLISECONDS}.<br />
+ * If this truncation would result in making the value zero when it was
specified as non-zero, then a minimum value of one {@link
TimeUnit#MILLISECONDS} will
+ * be used.
+ *
+ * <p>
+ * <b>Default:</b> {@link Long#MAX_VALUE} (no timeout)
*
* @param timeout
- * The maximum amount of time an unresponsive server will be
retried. When this timeout is exceeded, the BatchWriter should throw an
exception. For
- * no timeout set to zero or Long.MAX_VALUE with TimeUnit.MILLIS.
Defaults to no timeout.
+ * the timeout, in the unit specified by the value of {@code
timeUnit}
* @param timeUnit
- * @return this to allow chaining of set methods
+ * determines how {@code timeout} will be interpreted
+ * @throws IllegalArgumentException
+ * if {@code timeout} is less than 0
+ * @return {@code this} to allow chaining of set methods
*/
-
public BatchWriterConfig setTimeout(long timeout, TimeUnit timeUnit) {
if (timeout < 0)
throw new IllegalArgumentException("Negative timeout not allowed " +
timeout);
@@ -93,16 +123,23 @@ public class BatchWriterConfig implement
if (timeout == 0)
timeout = Long.MAX_VALUE;
else
- this.timeout = timeUnit.toMillis(timeout);
+ // make small, positive values that truncate to 0 when converted use the
minimum millis instead
+ this.timeout = Math.max(1, timeUnit.toMillis(timeout));
return this;
}
/**
+ * Sets the maximum number of threads to use for writing data to the tablet
servers.
+ *
+ * <p>
+ * <b>Default:</b> 3
+ *
* @param maxWriteThreads
- * the maximum number of threads to use for writing data to the
tablet servers. Defaults to 3.
- * @return this to allow chaining of set methods
+ * the maximum threads to use
+ * @throws IllegalArgumentException
+ * if {@code maxWriteThreads} is non-positive
+ * @return {@code this} to allow chaining of set methods
*/
-
public BatchWriterConfig setMaxWriteThreads(int maxWriteThreads) {
if (maxWriteThreads <= 0)
throw new IllegalArgumentException("Max threads must be positive " +
maxWriteThreads);
Modified:
accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java
URL:
http://svn.apache.org/viewvc/accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java?rev=1438248&r1=1438247&r2=1438248&view=diff
==============================================================================
---
accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java
(original)
+++
accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/BatchWriterConfigTest.java
Thu Jan 24 22:51:06 2013
@@ -63,19 +63,21 @@ public class BatchWriterConfigTest {
}
@Test
- public void testZeroTimeoutAndLatency() {
+ public void testZeroValues() {
BatchWriterConfig bwConfig = new BatchWriterConfig();
bwConfig.setMaxLatency(0, TimeUnit.MILLISECONDS);
bwConfig.setTimeout(0, TimeUnit.MILLISECONDS);
+ bwConfig.setMaxMemory(0);
assertEquals(Long.MAX_VALUE,
bwConfig.getMaxLatency(TimeUnit.MILLISECONDS));
assertEquals(Long.MAX_VALUE, bwConfig.getTimeout(TimeUnit.MILLISECONDS));
+ assertEquals(0, bwConfig.getMaxMemory());
}
@Test(expected = IllegalArgumentException.class)
- public void testMaxMemoryTooLow() {
+ public void testNegativeMaxMemory() {
BatchWriterConfig bwConfig = new BatchWriterConfig();
- bwConfig.setMaxMemory(1024 - 1);
+ bwConfig.setMaxMemory(-1);
}
@Test(expected = IllegalArgumentException.class)
@@ -84,6 +86,27 @@ public class BatchWriterConfigTest {
bwConfig.setMaxLatency(-1, TimeUnit.DAYS);
}
+ @Test
+ public void testTinyTimeConversions() {
+ BatchWriterConfig bwConfig = new BatchWriterConfig();
+ bwConfig.setMaxLatency(999, TimeUnit.MICROSECONDS);
+ bwConfig.setTimeout(999, TimeUnit.MICROSECONDS);
+
+ assertEquals(1000, bwConfig.getMaxLatency(TimeUnit.MICROSECONDS));
+ assertEquals(1000, bwConfig.getTimeout(TimeUnit.MICROSECONDS));
+ assertEquals(1, bwConfig.getMaxLatency(TimeUnit.MILLISECONDS));
+ assertEquals(1, bwConfig.getTimeout(TimeUnit.MILLISECONDS));
+
+ bwConfig.setMaxLatency(10, TimeUnit.NANOSECONDS);
+ bwConfig.setTimeout(10, TimeUnit.NANOSECONDS);
+
+ assertEquals(1000000, bwConfig.getMaxLatency(TimeUnit.NANOSECONDS));
+ assertEquals(1000000, bwConfig.getTimeout(TimeUnit.NANOSECONDS));
+ assertEquals(1, bwConfig.getMaxLatency(TimeUnit.MILLISECONDS));
+ assertEquals(1, bwConfig.getTimeout(TimeUnit.MILLISECONDS));
+
+ }
+
@Test(expected = IllegalArgumentException.class)
public void testNegativeTimeout() {
BatchWriterConfig bwConfig = new BatchWriterConfig();
