saxenapranav commented on code in PR #5446:
URL: https://github.com/apache/hadoop/pull/5446#discussion_r1127328623
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -268,10 +269,62 @@
DefaultValue = DEFAULT_ACCOUNT_OPERATION_IDLE_TIMEOUT_MS)
private int accountOperationIdleTimeout;
+ /**
+ * Analysis Period for client-side throttling
+ */
@IntegerConfigurationValidatorAnnotation(ConfigurationKey =
FS_AZURE_ANALYSIS_PERIOD,
DefaultValue = DEFAULT_ANALYSIS_PERIOD_MS)
private int analysisPeriod;
+ /**
+ * Lower limit of acceptable error percentage
+ */
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey =
FS_AZURE_MIN_ACCEPTABLE_ERROR_PERCENTAGE,
+ DefaultValue = DEFAULT_MIN_ACCEPTABLE_ERROR_PERCENTAGE)
+ private double minAcceptableErrorPercentage;
+
+ /**
+ * Maximum equilibrium error percentage
+ */
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey =
FS_AZURE_MAX_EQUILIBRIUM_ERROR_PERCENTAGE,
+ DefaultValue = DEFAULT_MAX_EQUILIBRIUM_ERROR_PERCENTAGE)
+ private double maxEquilibriumErrorPercentage;
+
+ /**
+ * Rapid sleep decrease factor to increase throughput
+ */
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey =
FS_AZURE_RAPID_SLEEP_DECREASE_FACTOR,
+ DefaultValue = DEFAULT_RAPID_SLEEP_DECREASE_FACTOR)
+ private double rapidSleepDecreaseFactor;
+
+ /**
+ * Rapid sleep decrease transition period in milliseconds
+ */
+ @DoubleConfigurationValidatorAnnotation(ConfigurationKey =
FS_AZURE_RAPID_SLEEP_DECREASE_TRANSITION_MS,
+ DefaultValue = DEFAULT_RAPID_SLEEP_DECREASE_TRANSITION_PERIOD_MS)
+ private double rapidSleepDecreaseTransitionPeriodMs;
+
+ /**
+ * Sleep decrease factor to increase throughput
+ */
Review Comment:
fix spacing in all javadocs wherever required.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -20,28 +20,42 @@
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
+import static java.net.HttpURLConnection.HTTP_OK;
+import static
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_GET;
+import static
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_PUT;
import static
org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_BACKOFF_INTERVAL;
import static
org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MAX_BACKOFF_INTERVAL;
import static
org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MAX_IO_RETRIES;
import static
org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_MIN_BACKOFF_INTERVAL;
import static
org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_LEVEL_THROTTLING_ENABLED;
import static
org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ENABLE_AUTOTHROTTLING;
import static
org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.MIN_BUFFER_SIZE;
+import static
org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.IF_MATCH;
+import static
org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.RANGE;
import static
org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_ABFS_ACCOUNT1_NAME;
import static
org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_ACCOUNT_NAME;
import static
org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.TEST_CONFIGURATION_FILE_NAME;
import static org.junit.Assume.assumeTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
Review Comment:
fix wildcard. You can switch off wildcard imports on IDE so in future it
doesnt come.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestExponentialRetryPolicy.java:
##########
@@ -285,6 +299,154 @@ public void testAbfsConfigConstructor() throws Exception {
Assert.assertEquals("Delta backoff interval was not set as expected.",
expectedDeltaBackoff, policy.getDeltaBackoff());
}
+// public void testClientBackoffOnlyNewRequest() throws IOException {
+ @Test
+ public void testClientBackoffOnlyNewWriteRequest() throws IOException,
InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = HTTP_METHOD_PUT;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.Append;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType,
client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long writeThrottleStatBefore =
counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0,
testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long writeThrottleStatAfter =
counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(new Long(writeThrottleStatBefore+1), writeThrottleStatAfter);
+
+
+ writeThrottleStatBefore =
counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType,
counters);
+ assertEquals(false, appliedBackoff);
+ writeThrottleStatAfter =
counters.toMap().get(AbfsStatistic.WRITE_THROTTLES.getStatName());
+ assertEquals(writeThrottleStatBefore, writeThrottleStatAfter);
+ }
+
+ @Test
+ public void testClientBackoffOnlyNewReadRequest() throws IOException,
InterruptedException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = fs.getAbfsStore().getClient();
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ URL dummyUrl = client.createRequestUrl("/", "");
+ String dummyMethod = AbfsHttpConstants.HTTP_METHOD_GET;
+
+ AbfsRestOperationType testOperationType = AbfsRestOperationType.ReadFile;
+
+ AbfsRestOperation restOp = new AbfsRestOperation(testOperationType,
client, dummyMethod, dummyUrl, new ArrayList<>());
+
+ Long readThrottleStatBefore =
counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ Thread.sleep(10000);
+ boolean appliedBackoff = restOp.applyThrottlingBackoff(0,
testOperationType, counters);
+ assertEquals(true, appliedBackoff);
+ Long readThrottleStatAfter =
counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(new Long(readThrottleStatBefore+1), readThrottleStatAfter);
+
+
+ readThrottleStatBefore =
counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ appliedBackoff = restOp.applyThrottlingBackoff(1, testOperationType,
counters);
+ assertEquals(false, appliedBackoff);
+ readThrottleStatAfter =
counters.toMap().get(AbfsStatistic.READ_THROTTLES.getStatName());
+ assertEquals(readThrottleStatBefore, readThrottleStatAfter);
+ }
+
+ @Test
+ public void testReadThrottleNewRequest() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient());
+ AbfsConfiguration configuration = client.getAbfsConfiguration();
+ Assume.assumeTrue(configuration.isAutoThrottlingEnabled());
+ AbfsCounters counters = client.getAbfsCounters();
+
+ AbfsThrottlingIntercept intercept =
Mockito.mock(AbfsThrottlingIntercept.class);
+
Mockito.doNothing().when(intercept).sendingRequest(Mockito.any(AbfsRestOperationType.class),
Mockito.any(AbfsCounters.class));
+ Mockito.doReturn(intercept).when(client).getIntercept();
+
+ // setting up the spy AbfsRestOperation class for read
+ final List<AbfsHttpHeader> requestHeaders = client.createDefaultHeaders();
+
+ final AbfsUriQueryBuilder abfsUriQueryBuilder =
client.createDefaultUriQueryBuilder();
+
+ final URL url = client.createRequestUrl("/dummyReadFile",
abfsUriQueryBuilder.toString());
+ final AbfsRestOperation mockRestOp = Mockito.spy(new AbfsRestOperation(
+ AbfsRestOperationType.ReadFile,
+ client,
+ HTTP_METHOD_GET,
+ url,
+ requestHeaders));
+
+ // setting up mock behavior for the AbfsHttpOperation class
+ AbfsHttpOperation mockHttpOp =
Mockito.spy(mockRestOp.createHttpOperationInstance());
Review Comment:
lets resolve to not do this. Some issue in our mocking. The way described
should work.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -222,6 +224,10 @@ AbfsThrottlingIntercept getIntercept() {
return intercept;
}
+ boolean shouldThrottleRetries() {
+ return throttleRetries;
+ }
+
Review Comment:
why is it in abfsClient. What risk is there to keep it in abfsRestOperation.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
