Skip to content

Fixes #3630 - ClientContext updated to handle time duration values correctly #3631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dlmarion merged 1 commit into from
Jul 20, 2023
Merged

Fixes #3630 - ClientContext updated to handle time duration values correctly #3631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,11 @@ static BatchWriterConfig getBatchWriterConfig(Properties props) {
}
Long maxLatency = ClientProperty.BATCH_WRITER_LATENCY_MAX.getTimeInMillis(props);
if (maxLatency != null) {
batchWriterConfig.setMaxLatency(maxLatency, SECONDS);
batchWriterConfig.setMaxLatency(maxLatency, MILLISECONDS);
}
Long timeout = ClientProperty.BATCH_WRITER_TIMEOUT_MAX.getTimeInMillis(props);
if (timeout != null) {
batchWriterConfig.setTimeout(timeout, SECONDS);
batchWriterConfig.setTimeout(timeout, MILLISECONDS);
}
Integer maxThreads = ClientProperty.BATCH_WRITER_THREADS_MAX.getInteger(props);
if (maxThreads != null) {
Expand Down Expand Up @@ -433,7 +433,7 @@ static ConditionalWriterConfig getConditionalWriterConfig(Properties props) {

Long timeout = ClientProperty.CONDITIONAL_WRITER_TIMEOUT_MAX.getTimeInMillis(props);
if (timeout != null) {
conditionalWriterConfig.setTimeout(timeout, SECONDS);
conditionalWriterConfig.setTimeout(timeout, MILLISECONDS);
}
String durability = ClientProperty.CONDITIONAL_WRITER_DURABILITY.getValue(props);
if (!durability.isEmpty()) {
Expand Down
43 changes: 18 additions & 25 deletions core/src/test/java/org/apache/accumulo/core/clientImpl/ClientContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,26 +111,21 @@ public void testGetBatchWriterConfigUsingDefaults() {
// 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(MILLISECONDS));
} else {
assertEquals(expectedLatency, batchWriterConfig.getMaxLatency(SECONDS));
}

// default latency should be 120000 ms
assertEquals(120000L, expectedLatency);
assertEquals(expectedLatency, batchWriterConfig.getMaxLatency(MILLISECONDS));

long expectedTimeout = ConfigurationTypeHelper
.getTimeInMillis(ClientProperty.BATCH_WRITER_TIMEOUT_MAX.getDefaultValue());
if (expectedTimeout == 0) {
expectedTimeout = Long.MAX_VALUE;
assertEquals(expectedTimeout, batchWriterConfig.getTimeout(MILLISECONDS));
} else {
assertEquals(expectedTimeout, batchWriterConfig.getTimeout(SECONDS));
}
assertEquals(expectedTimeout, Long.MAX_VALUE);
assertEquals(expectedTimeout, batchWriterConfig.getTimeout(MILLISECONDS));

int expectedThreads =
Integer.parseInt(ClientProperty.BATCH_WRITER_THREADS_MAX.getDefaultValue());
Expand All @@ -147,7 +142,7 @@ public void testGetBatchWriterConfigNotUsingDefaults() {

// 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_LATENCY_MAX.getKey(), "40");
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());
Expand All @@ -159,10 +154,11 @@ public void testGetBatchWriterConfigNotUsingDefaults() {
.getMemoryAsBytes(ClientProperty.BATCH_WRITER_MEMORY_MAX.getValue(props));
assertEquals(expectedMemory, batchWriterConfig.getMaxMemory());

assertEquals(Long.MAX_VALUE, batchWriterConfig.getMaxLatency(MILLISECONDS));
assertEquals(40, batchWriterConfig.getMaxLatency(SECONDS));
assertEquals(40000, batchWriterConfig.getMaxLatency(MILLISECONDS));

// getTimeout returns time in milliseconds, therefore the 15 becomes 15000.
assertEquals(15000, batchWriterConfig.getTimeout(SECONDS));
assertEquals(15, batchWriterConfig.getTimeout(SECONDS));
assertEquals(15000, batchWriterConfig.getTimeout(MILLISECONDS));

long expectedThreads = ClientProperty.BATCH_WRITER_THREADS_MAX.getInteger(props);
assertEquals(expectedThreads, batchWriterConfig.getMaxWriteThreads());
Expand All @@ -180,17 +176,14 @@ public void testGetConditionalWriterConfigUsingDefaults() {
assertNotNull(conditionalWriterConfig);

// If the value of CONDITIONAL_WRITER_TIMEOUT_MAX is set to zero, Long.MAX_VALUE is returned.
// Effectively, this indicates there is no timeout for CONDITIONAL_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.
// Effectively, this indicates there is no timeout for CONDITIONAL_WRITER_TIMEOUT_MAX
long expectedTimeout = ConfigurationTypeHelper
.getTimeInMillis(ClientProperty.CONDITIONAL_WRITER_TIMEOUT_MAX.getDefaultValue());
if (expectedTimeout == 0) {
assertEquals(Long.MAX_VALUE, conditionalWriterConfig.getTimeout(MILLISECONDS));
} else {
assertEquals(expectedTimeout, conditionalWriterConfig.getTimeout(SECONDS));
expectedTimeout = Long.MAX_VALUE;
}
assertEquals(expectedTimeout, Long.MAX_VALUE);
assertEquals(expectedTimeout, conditionalWriterConfig.getTimeout(MILLISECONDS));

int expectedThreads =
Integer.parseInt(ClientProperty.CONDITIONAL_WRITER_THREADS_MAX.getDefaultValue());
Expand All @@ -215,8 +208,8 @@ public void testGetConditionalWriterConfigNotUsingDefaults() {
ClientContext.getConditionalWriterConfig(props);
assertNotNull(conditionalWriterConfig);

// getTimeout returns time in milliseconds, therefore the 17 becomes 17000.
assertEquals(17000, conditionalWriterConfig.getTimeout(SECONDS));
assertEquals(17, conditionalWriterConfig.getTimeout(SECONDS));
assertEquals(17000, conditionalWriterConfig.getTimeout(MILLISECONDS));

long expectedThreads = ClientProperty.CONDITIONAL_WRITER_THREADS_MAX.getInteger(props);
assertEquals(expectedThreads, conditionalWriterConfig.getMaxWriteThreads());
Expand Down