Skip to content

Commit 44765c0

Browse files
committed
Addressed comments
1 parent bb93023 commit 44765c0

File tree

9 files changed

+66
-5
lines changed

9 files changed

+66
-5
lines changed

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,10 @@ public class AbfsConfiguration{
203203
DefaultValue = DEFAULT_HTTP_READ_TIMEOUT)
204204
private int httpReadTimeout;
205205

206+
@IntegerConfigurationValidatorAnnotation(ConfigurationKey = AZURE_EXPECT_100CONTINUE_WAIT_TIMEOUT,
207+
DefaultValue = DEFAULT_EXPECT_100CONTINUE_WAIT_TIMEOUT)
208+
private int expect100ContinueWaitTimeout;
209+
206210
@IntegerConfigurationValidatorAnnotation(ConfigurationKey = AZURE_OAUTH_TOKEN_FETCH_RETRY_COUNT,
207211
MinValue = 0,
208212
DefaultValue = DEFAULT_AZURE_OAUTH_TOKEN_FETCH_RETRY_MAX_ATTEMPTS)
@@ -1033,6 +1037,10 @@ public int getHttpReadTimeout() {
10331037
return this.httpReadTimeout;
10341038
}
10351039

1040+
public int getExpect100ContinueWaitTimeout() {
1041+
return this.expect100ContinueWaitTimeout;
1042+
}
1043+
10361044
public long getAzureBlockSize() {
10371045
return this.azureBlockSize;
10381046
}

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/ConfigurationKeys.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,13 @@ public final class ConfigurationKeys {
9797
*/
9898
public static final String AZURE_HTTP_READ_TIMEOUT = "fs.azure.http.read.timeout";
9999

100+
/**
101+
* Config to set HTTP Expect100-Continue Wait Timeout Value for Rest Operations.
102+
* Value: {@value}.
103+
*/
104+
public static final String AZURE_EXPECT_100CONTINUE_WAIT_TIMEOUT
105+
= "fs.azure.http.expect.100continue.wait.timeout";
106+
100107
// Retry strategy for getToken calls
101108
public static final String AZURE_OAUTH_TOKEN_FETCH_RETRY_COUNT = "fs.azure.oauth.token.fetch.retry.max.retries";
102109
public static final String AZURE_OAUTH_TOKEN_FETCH_RETRY_MIN_BACKOFF = "fs.azure.oauth.token.fetch.retry.min.backoff.interval";

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ public final class FileSystemConfigurations {
5858
*/
5959
public static final int DEFAULT_HTTP_READ_TIMEOUT = 30_000; // 30 secs
6060

61+
/**
62+
* Default value of connection request timeout to be used when 100continue is enabled.
63+
* Value: {@value}.
64+
*/
65+
public static final int DEFAULT_EXPECT_100CONTINUE_WAIT_TIMEOUT = 3_000; // 3s
66+
6167
// Retry parameter defaults.
6268
public static final int DEFAULT_AZURE_OAUTH_TOKEN_FETCH_RETRY_MAX_ATTEMPTS = 5;
6369
public static final int DEFAULT_AZURE_OAUTH_TOKEN_FETCH_RETRY_MIN_BACKOFF_INTERVAL = 0;

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsApacheHttpClient.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,14 @@ static boolean usable() {
9393
final HttpClientBuilder builder = HttpClients.custom();
9494
builder.setConnectionManager(connMgr)
9595
.setRequestExecutor(
96+
// In case of Expect:100-continue, the timeout for waiting for
97+
// the 100-continue response from the server is set using
98+
// ExpectWaitContinueTimeout. For other requests, the read timeout
99+
// is set using SocketTimeout.
96100
new AbfsManagedHttpRequestExecutor(
97-
abfsConfiguration.getHttpReadTimeout()))
101+
abfsConfiguration.isExpectHeaderEnabled()
102+
? abfsConfiguration.getExpect100ContinueWaitTimeout()
103+
: abfsConfiguration.getHttpReadTimeout()))
98104
.disableContentCompression()
99105
.disableRedirectHandling()
100106
.disableAutomaticRetries()

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,9 @@ private AbfsClient(final URL baseUrl,
257257
== HttpOperationType.APACHE_HTTP_CLIENT) {
258258
keepAliveCache = new KeepAliveCache(abfsConfiguration);
259259

260+
// Warm up the connection pool during client initialization to avoid latency during first request.
261+
// Since for every filesystem instance, we create both DFS and Blob client instance,
262+
// so warmup is done only for the default client.
260263
abfsApacheHttpClient = new AbfsApacheHttpClient(
261264
DelegatingSSLSocketFactory.getDefaultFactory(),
262265
abfsConfiguration, keepAliveCache, baseUrl,

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientHandler.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ public AbfsClientHandler(final URL baseUrl,
6868
final SASTokenProvider sasTokenProvider,
6969
final EncryptionContextProvider encryptionContextProvider,
7070
final AbfsClientContext abfsClientContext) throws IOException {
71+
// This will initialize the default and ingress service types.
72+
// This is needed before crating the clients so that we can do cache warmup
73+
// only for default client.
7174
initServiceType(abfsConfiguration);
7275
this.dfsAbfsClient = createDfsClient(baseUrl, sharedKeyCredentials,
7376
abfsConfiguration, null, sasTokenProvider, encryptionContextProvider,

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsConnectionManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ private int cacheExtraConnection(final HttpRoute route,
361361
} catch (RejectedExecutionException e) {
362362
LOG.debug("Task rejected for connection creation: {}",
363363
e.getMessage());
364-
return 0;
364+
return -1;
365365
}
366366
}
367367

hadoop-tools/hadoop-azure/src/site/markdown/index.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -890,8 +890,8 @@ ABFS Driver can use the following networking libraries:
890890
The networking library can be configured using the configuration `fs.azure.networking.library`
891891
while initializing the filesystem.
892892
Following are the supported values:
893-
- `JDK_HTTP_URL_CONNECTION` : Use JDK networking library [Default]
894-
- `APACHE_HTTP_CLIENT` : Use Apache HttpClient
893+
- `JDK_HTTP_URL_CONNECTION` : Use JDK networking library
894+
- `APACHE_HTTP_CLIENT` : Use Apache HttpClient [Default]
895895

896896
#### <a href="ahc_networking_conf"></a>ApacheHttpClient networking layer configuration Options:
897897

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestApacheClientConnectionPool.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest;
3434
import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem;
3535
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsDriverException;
36+
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
3637
import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory;
3738
import org.apache.hadoop.util.functional.Tuples;
3839
import org.apache.http.HttpHost;
@@ -45,8 +46,10 @@
4546
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
4647
import org.apache.http.impl.conn.DefaultHttpClientConnectionOperator;
4748

49+
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APACHE_IMPL;
4850
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.COLON;
4951
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EMPTY_STRING;
52+
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.JDK_FALLBACK;
5053
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.KEEP_ALIVE_CACHE_CLOSED;
5154
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_FORMAT;
5255
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_NETWORKING_LIBRARY;
@@ -74,7 +77,9 @@ public void testKacIsClosed() throws Throwable {
7477
configuration.unset(FS_AZURE_METRIC_FORMAT);
7578
try (AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(
7679
configuration)) {
77-
KeepAliveCache kac = fs.getAbfsStore().getClientHandler().getIngressClient()
80+
KeepAliveCache kac = fs.getAbfsStore()
81+
.getClientHandler()
82+
.getIngressClient()
7883
.getKeepAliveCache();
7984
kac.close();
8085
AbfsDriverException ex = intercept(AbfsDriverException.class,
@@ -149,10 +154,33 @@ public void testApacheClientFallbackDuringConnectionWarmup()
149154
Assertions.assertThat(AbfsApacheHttpClient.usable())
150155
.describedAs("Apache HttpClient should be not usable")
151156
.isFalse();
157+
// Make a rest API call to verify that the client falls back to JDK client.
158+
AzureBlobFileSystem fs = getFileSystem();
159+
verifyClientRequestId(fs, JDK_FALLBACK);
152160
AbfsApacheHttpClient.setUsable();
161+
verifyClientRequestId(fs, APACHE_IMPL);
153162
}
154163
}
155164

165+
/**
166+
* Verify that the client request id contains the expected client.
167+
* @param fs AzureBlobFileSystem instance
168+
* @param expectedClient Expected client in the client request id.
169+
* @throws AzureBlobFileSystemException if any failure occurs during the operation.
170+
*/
171+
private void verifyClientRequestId(AzureBlobFileSystem fs,
172+
String expectedClient)
173+
throws AzureBlobFileSystemException {
174+
AbfsRestOperation op = fs.getAbfsStore()
175+
.getClient()
176+
.getFilesystemProperties(getTestTracingContext(fs, true));
177+
String[] clientRequestIdList = op.getResult()
178+
.getClientRequestId().split(COLON);
179+
Assertions.assertThat(clientRequestIdList[clientRequestIdList.length - 1])
180+
.describedAs("Http Client in use should be %s", expectedClient)
181+
.isEqualTo(expectedClient);
182+
}
183+
156184
private Map.Entry<HttpRoute, AbfsManagedApacheHttpConnection> getTestConnection()
157185
throws IOException {
158186
HttpHost host = new HttpHost(getFileSystem().getUri().getHost(),

0 commit comments

Comments
 (0)