keith-turner commented on a change in pull request #2126:
URL: https://github.com/apache/accumulo/pull/2126#discussion_r643514748
##########
File path:
core/src/test/java/org/apache/accumulo/core/clientImpl/ClientContextTest.java
##########
@@ -90,4 +95,96 @@ public void sensitivePropertiesIncludedInProperties() {
assertFalse(props.containsKey("ignored.property"));
assertEquals("mysecret", props.get(Property.INSTANCE_SECRET.getKey()));
}
+
+ @Test
+ public void testGetBatchWriterConfigUsingDefaults() {
+ Properties props = new Properties();
+ BatchWriterConfig batchWriterConfig =
ClientContext.getBatchWriterConfig(props);
+ assertNotNull(batchWriterConfig);
+
+ long expectedMemory = ConfigurationTypeHelper
+
.getMemoryAsBytes(ClientProperty.BATCH_WRITER_MEMORY_MAX.getDefaultValue());
+ assertEquals(expectedMemory, batchWriterConfig.getMaxMemory());
+
+ // If the value of BATCH_WRITE_LATENCY_MAX or BATCH_WRITER_TIMEOUT_MAX, is
set to zero,
+ // Long.MAX_VALUE is returned. Effectively, this will cause data to be
held in memory
+ // indefinitely for BATCH_WRITE_LATENCY_MAX and for no timeout, for
BATCH_WRITER_TIMEOUT_MAX.
+ // Due to this behavior, the test compares the return values differently.
If a value of
+ // 0 is used, compare the return value using TimeUnit.MILLISECONDS,
otherwise the value
+ // should be converted to seconds in order to match the value set in
ClientProperty.
+ long expectedLatency = ConfigurationTypeHelper
+
.getTimeInMillis(ClientProperty.BATCH_WRITER_LATENCY_MAX.getDefaultValue());
+ if (expectedLatency == 0) {
+ expectedLatency = Long.MAX_VALUE;
+ assertEquals(expectedLatency,
batchWriterConfig.getMaxLatency(TimeUnit.MILLISECONDS));
+ } else {
+ assertEquals(expectedLatency,
batchWriterConfig.getMaxLatency(TimeUnit.SECONDS));
+ }
+
+ long expectedTimeout = ConfigurationTypeHelper
+
.getTimeInMillis(ClientProperty.BATCH_WRITER_TIMEOUT_MAX.getDefaultValue());
+ if (expectedTimeout == 0) {
+ expectedTimeout = Long.MAX_VALUE;
+ assertEquals(expectedTimeout,
batchWriterConfig.getTimeout(TimeUnit.MILLISECONDS));
+ } else {
+ assertEquals(expectedTimeout,
batchWriterConfig.getTimeout(TimeUnit.SECONDS));
+ }
+
+ int expectedThreads =
+
Integer.parseInt(ClientProperty.BATCH_WRITER_THREADS_MAX.getDefaultValue());
+ assertEquals(expectedThreads, batchWriterConfig.getMaxWriteThreads());
+
+ Durability expectedDurability =
+
Durability.valueOf(ClientProperty.BATCH_WRITER_DURABILITY.getDefaultValue().toUpperCase());
+ assertEquals(expectedDurability, batchWriterConfig.getDurability());
+ }
+
+ @Test
+ public void testGetBatchWriterConfigNotUsingDefaults() {
+ Properties props = new Properties();
+
+ // set properties to non-default values
+ props.setProperty(ClientProperty.BATCH_WRITER_MEMORY_MAX.getKey(), "10M");
+ props.setProperty(ClientProperty.BATCH_WRITER_LATENCY_MAX.getKey(), "0");
+ props.setProperty(ClientProperty.BATCH_WRITER_TIMEOUT_MAX.getKey(), "15");
+ props.setProperty(ClientProperty.BATCH_WRITER_THREADS_MAX.getKey(), "12");
+ props.setProperty(ClientProperty.BATCH_WRITER_DURABILITY.getKey(),
Durability.FLUSH.name());
+
+ BatchWriterConfig batchWriterConfig =
ClientContext.getBatchWriterConfig(props);
+ assertNotNull(batchWriterConfig);
+
+ long expectedMemory = ConfigurationTypeHelper
+
.getMemoryAsBytes(ClientProperty.BATCH_WRITER_MEMORY_MAX.getValue(props));
+ assertEquals(expectedMemory, batchWriterConfig.getMaxMemory());
+
+ // If the value of BATCH_WRITE_LATENCY_MAX or BATCH_WRITER_TIMEOUT_MAX, is
set to zero,
+ // Long.MAX_VALUE is returned. Effectively, this will cause data to be
held in memory
+ // indefinitely for BATCH_WRITE_LATENCY_MAX and for no timeout, for
BATCH_WRITER_TIMEOUT_MAX.
+ // Due to this behavior, the test compares the return values differently.
If a value of
+ // 0 is used, compare the return value using TimeUnit.MILLISECONDS,
otherwise the value
+ // should be converted to seconds in order to match the value set in
ClientProperty.
+ long expectedLatency =
ClientProperty.BATCH_WRITER_LATENCY_MAX.getTimeInMillis(props);
+ if (expectedLatency == 0) {
+ expectedLatency = Long.MAX_VALUE;
+ assertEquals(expectedLatency,
batchWriterConfig.getMaxLatency(TimeUnit.MILLISECONDS));
+ } else {
+ assertEquals(expectedLatency,
batchWriterConfig.getMaxLatency(TimeUnit.SECONDS));
+ }
Review comment:
Seems like since we know what the prop was set to the we can just check
for that.
```suggestion
assertEquals(Long.MAX_VALUE,
batchWriterConfig.getMaxLatency(TimeUnit.MILLISECONDS));
```
##########
File path:
core/src/test/java/org/apache/accumulo/core/clientImpl/ClientContextTest.java
##########
@@ -90,4 +95,96 @@ public void sensitivePropertiesIncludedInProperties() {
assertFalse(props.containsKey("ignored.property"));
assertEquals("mysecret", props.get(Property.INSTANCE_SECRET.getKey()));
}
+
+ @Test
+ public void testGetBatchWriterConfigUsingDefaults() {
+ Properties props = new Properties();
+ BatchWriterConfig batchWriterConfig =
ClientContext.getBatchWriterConfig(props);
+ assertNotNull(batchWriterConfig);
+
+ long expectedMemory = ConfigurationTypeHelper
+
.getMemoryAsBytes(ClientProperty.BATCH_WRITER_MEMORY_MAX.getDefaultValue());
+ assertEquals(expectedMemory, batchWriterConfig.getMaxMemory());
+
+ // If the value of BATCH_WRITE_LATENCY_MAX or BATCH_WRITER_TIMEOUT_MAX, is
set to zero,
+ // Long.MAX_VALUE is returned. Effectively, this will cause data to be
held in memory
+ // indefinitely for BATCH_WRITE_LATENCY_MAX and for no timeout, for
BATCH_WRITER_TIMEOUT_MAX.
+ // Due to this behavior, the test compares the return values differently.
If a value of
+ // 0 is used, compare the return value using TimeUnit.MILLISECONDS,
otherwise the value
+ // should be converted to seconds in order to match the value set in
ClientProperty.
+ long expectedLatency = ConfigurationTypeHelper
+
.getTimeInMillis(ClientProperty.BATCH_WRITER_LATENCY_MAX.getDefaultValue());
+ if (expectedLatency == 0) {
+ expectedLatency = Long.MAX_VALUE;
+ assertEquals(expectedLatency,
batchWriterConfig.getMaxLatency(TimeUnit.MILLISECONDS));
+ } else {
+ assertEquals(expectedLatency,
batchWriterConfig.getMaxLatency(TimeUnit.SECONDS));
+ }
+
+ long expectedTimeout = ConfigurationTypeHelper
+
.getTimeInMillis(ClientProperty.BATCH_WRITER_TIMEOUT_MAX.getDefaultValue());
+ if (expectedTimeout == 0) {
+ expectedTimeout = Long.MAX_VALUE;
+ assertEquals(expectedTimeout,
batchWriterConfig.getTimeout(TimeUnit.MILLISECONDS));
+ } else {
+ assertEquals(expectedTimeout,
batchWriterConfig.getTimeout(TimeUnit.SECONDS));
+ }
+
+ int expectedThreads =
+
Integer.parseInt(ClientProperty.BATCH_WRITER_THREADS_MAX.getDefaultValue());
+ assertEquals(expectedThreads, batchWriterConfig.getMaxWriteThreads());
+
+ Durability expectedDurability =
+
Durability.valueOf(ClientProperty.BATCH_WRITER_DURABILITY.getDefaultValue().toUpperCase());
+ assertEquals(expectedDurability, batchWriterConfig.getDurability());
+ }
+
+ @Test
+ public void testGetBatchWriterConfigNotUsingDefaults() {
+ Properties props = new Properties();
+
+ // set properties to non-default values
+ props.setProperty(ClientProperty.BATCH_WRITER_MEMORY_MAX.getKey(), "10M");
+ props.setProperty(ClientProperty.BATCH_WRITER_LATENCY_MAX.getKey(), "0");
+ props.setProperty(ClientProperty.BATCH_WRITER_TIMEOUT_MAX.getKey(), "15");
+ props.setProperty(ClientProperty.BATCH_WRITER_THREADS_MAX.getKey(), "12");
+ props.setProperty(ClientProperty.BATCH_WRITER_DURABILITY.getKey(),
Durability.FLUSH.name());
+
+ BatchWriterConfig batchWriterConfig =
ClientContext.getBatchWriterConfig(props);
+ assertNotNull(batchWriterConfig);
+
+ long expectedMemory = ConfigurationTypeHelper
+
.getMemoryAsBytes(ClientProperty.BATCH_WRITER_MEMORY_MAX.getValue(props));
+ assertEquals(expectedMemory, batchWriterConfig.getMaxMemory());
+
+ // If the value of BATCH_WRITE_LATENCY_MAX or BATCH_WRITER_TIMEOUT_MAX, is
set to zero,
+ // Long.MAX_VALUE is returned. Effectively, this will cause data to be
held in memory
+ // indefinitely for BATCH_WRITE_LATENCY_MAX and for no timeout, for
BATCH_WRITER_TIMEOUT_MAX.
+ // Due to this behavior, the test compares the return values differently.
If a value of
+ // 0 is used, compare the return value using TimeUnit.MILLISECONDS,
otherwise the value
+ // should be converted to seconds in order to match the value set in
ClientProperty.
+ long expectedLatency =
ClientProperty.BATCH_WRITER_LATENCY_MAX.getTimeInMillis(props);
+ if (expectedLatency == 0) {
+ expectedLatency = Long.MAX_VALUE;
+ assertEquals(expectedLatency,
batchWriterConfig.getMaxLatency(TimeUnit.MILLISECONDS));
+ } else {
+ assertEquals(expectedLatency,
batchWriterConfig.getMaxLatency(TimeUnit.SECONDS));
+ }
+
+ long expectedTimeout =
ClientProperty.BATCH_WRITER_TIMEOUT_MAX.getTimeInMillis(props);
+ if (expectedTimeout == 0) {
+ expectedTimeout = Long.MAX_VALUE;
+ assertEquals(expectedTimeout,
batchWriterConfig.getTimeout(TimeUnit.MILLISECONDS));
+ } else {
+ assertEquals(expectedTimeout,
batchWriterConfig.getTimeout(TimeUnit.SECONDS));
+ }
Review comment:
```suggestion
assertEquals(15, batchWriterConfig.getTimeout(TimeUnit.SECONDS));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]