bhattmanish98 commented on code in PR #8043:
URL: https://github.com/apache/hadoop/pull/8043#discussion_r2469106859
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java:
##########
@@ -266,5 +266,15 @@ public final class FileSystemConfigurations {
public static final boolean DEFAULT_FS_AZURE_ENABLE_CREATE_BLOB_IDEMPOTENCY
= true;
+ public static final boolean DEFAULT_FS_AZURE_ENABLE_TAIL_LATENCY_TRACKER =
false;
+ public static final boolean
DEFAULT_FS_AZURE_ENABLE_TAIL_LATENCY_REQUEST_TIMEOUT = false;
+ public static final int DEFAULT_FS_AZURE_TAIL_LATENCY_PERCENTILE = 99;
Review Comment:
shouldn't it be float/double instead of int? Tomorrow we can change default
percentile to 99.9 or 99.99.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/exceptions/TailLatencyRequestTimeoutException.java:
##########
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.contracts.exceptions;
+
+import java.util.concurrent.TimeoutException;
+import static
org.apache.hadoop.fs.azurebfs.services.AbfsErrors.ERR_TAIL_LATENCY_REQUEST_TIMEOUT;
+
+/**
+ * Thrown when a request takes more time than the current reported tail
latency.
+ */
+public class TailLatencyRequestTimeoutException extends
AzureBlobFileSystemException {
+
+ /**
+ * Constructs a TailLatencyRequestTimeoutException with TimeoutException as
the cause.
+ */
+ public TailLatencyRequestTimeoutException(TimeoutException innerException) {
Review Comment:
@param missing in the java doc
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsApacheHttpClient.java:
##########
@@ -132,6 +138,30 @@ public void close() throws IOException {
* @throws IOException network error.
*/
public HttpResponse execute(HttpRequestBase httpRequest,
+ final AbfsManagedHttpClientContext abfsHttpClientContext,
+ final int connectTimeout,
+ final int readTimeout,
+ final long tailLatencyTimeout) throws IOException {
+ if (tailLatencyTimeout <= 0) {
+ return executeWithoutDeadline(httpRequest, abfsHttpClientContext,
+ connectTimeout, readTimeout);
+ }
+ return executeWithDeadline(httpRequest, abfsHttpClientContext,
+ connectTimeout, readTimeout, tailLatencyTimeout);
+ }
+
+ /**
+ * Executes the HTTP request.
+ *
+ * @param httpRequest HTTP request to execute.
+ * @param abfsHttpClientContext HttpClient context.
+ * @param connectTimeout Connection timeout.
+ * @param readTimeout Read timeout.
+ *
+ * @return HTTP response.
+ * @throws IOException network error.
+ */
+ public HttpResponse executeWithoutDeadline(HttpRequestBase httpRequest,
Review Comment:
executeWithoutDeadline and executeWithDeadline can be private methods.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/exceptions/TailLatencyRequestTimeoutException.java:
##########
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.contracts.exceptions;
+
+import java.util.concurrent.TimeoutException;
+import static
org.apache.hadoop.fs.azurebfs.services.AbfsErrors.ERR_TAIL_LATENCY_REQUEST_TIMEOUT;
+
+/**
+ * Thrown when a request takes more time than the current reported tail
latency.
+ */
+public class TailLatencyRequestTimeoutException extends
AzureBlobFileSystemException {
+
+ /**
+ * Constructs a TailLatencyRequestTimeoutException with TimeoutException as
the cause.
+ */
+ public TailLatencyRequestTimeoutException(TimeoutException innerException) {
+ super(ERR_TAIL_LATENCY_REQUEST_TIMEOUT, innerException);
+ }
+
+ public TailLatencyRequestTimeoutException() {
Review Comment:
Java doc missing for this
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsTailLatencyTracker.java:
##########
@@ -0,0 +1,159 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.services;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.fs.azurebfs.AbfsConfiguration;
+
+/**
+ * Account Specific Latency Tracker.
+ * This class tracks the latency of various operations like read, write etc
for a single account.
+ * It maintains a sliding window histogram for each operation type to analyze
latency patterns over time.
+ */
+public class AbfsTailLatencyTracker {
+
+ private static final Logger LOG = LoggerFactory.getLogger(
+ AbfsTailLatencyTracker.class);
+ private static AbfsTailLatencyTracker singleton;
Review Comment:
We can rename this variable to something which is more relevant.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsTailLatencyTracker.java:
##########
@@ -0,0 +1,159 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.services;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.fs.azurebfs.AbfsConfiguration;
+
+/**
+ * Account Specific Latency Tracker.
+ * This class tracks the latency of various operations like read, write etc
for a single account.
+ * It maintains a sliding window histogram for each operation type to analyze
latency patterns over time.
+ */
+public class AbfsTailLatencyTracker {
+
+ private static final Logger LOG = LoggerFactory.getLogger(
+ AbfsTailLatencyTracker.class);
+ private static AbfsTailLatencyTracker singleton;
+ private static final ReentrantLock LOCK = new ReentrantLock();
+ private static final int HISTOGRAM_MAX_VALUE = 60_000;
+ private static final int HISTOGRAM_SIGNIFICANT_FIGURES = 3;
+ private final Map<AbfsRestOperationType, SlidingWindowHdrHistogram>
+ operationLatencyMap = new HashMap<>();
+ private final AbfsConfiguration configuration;
+
+ /**
+ * Constructor to initialize the latency tracker with configuration.
+ * @param abfsConfiguration Configuration settings for latency tracking.
+ */
+ public AbfsTailLatencyTracker(AbfsConfiguration abfsConfiguration) {
+ this.configuration = abfsConfiguration;
+ ScheduledExecutorService histogramRotatorThread =
Executors.newSingleThreadScheduledExecutor(
+ r -> {
+ Thread t = new Thread(r, "Histogram-Rotator-Thread");
+ t.setDaemon(true);
+ return t;
+ });
+ long rotationInterval =
configuration.getTailLatencyAnalysisWindowInMillis()
+ / configuration.getTailLatencyAnalysisWindowGranularity();
+ histogramRotatorThread.scheduleAtFixedRate(this::rotateHistograms,
+ rotationInterval, rotationInterval, TimeUnit.MILLISECONDS);
+
+
+ ScheduledExecutorService tailLatencyComputationThread =
Executors.newSingleThreadScheduledExecutor(
Review Comment:
We should close this thread pool and one below once the use is done or at
least during filesystem close.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java:
##########
@@ -611,10 +630,30 @@ AbfsJdkHttpOperation createAbfsHttpOperation() throws
IOException {
@VisibleForTesting
AbfsAHCHttpOperation createAbfsAHCHttpOperation() throws IOException {
+ long tailLatency = getTailLatencyTimeoutIfEnabled();
return new AbfsAHCHttpOperation(url, method, requestHeaders,
Duration.ofMillis(client.getAbfsConfiguration().getHttpConnectionTimeout()),
Duration.ofMillis(client.getAbfsConfiguration().getHttpReadTimeout()),
- client.getAbfsApacheHttpClient(), client);
+ tailLatency, client.getAbfsApacheHttpClient(), client);
+ }
+
+ /**
+ * Get Tail Latency Timeout value if profiling is enabled, timeout is enabled
+ * and retries due to tail latency request timeout is allowed.
+ * @return tail latency timeout value else return zero.
+ */
+ long getTailLatencyTimeoutIfEnabled() {
+ if (isTailLatencyTimeoutEnabled() && shouldTailLatencyTimeout) {
+ return (long) tailLatencyTracker.getTailLatency(this.operationType);
+ }
+ return ZERO;
+ }
+
+ boolean isTailLatencyTimeoutEnabled() {
Review Comment:
Java doc missing
--
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]
