steveloughran commented on code in PR #4907:
URL: https://github.com/apache/hadoop/pull/4907#discussion_r977634550
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientThrottlingIntercept.java:
##########
@@ -55,10 +55,10 @@ private synchronized void
setIsAutoThrottlingEnabled(boolean autoThrottlingEnabl
// Hide default constructor
public AbfsClientThrottlingIntercept(String accountName) {
setIsAutoThrottlingEnabled(true);
- LOG.debug("Client-side throttling is enabled for the ABFS file system.");
this.readThrottler = new AbfsClientThrottlingAnalyzer("read");
this.writeThrottler = new AbfsClientThrottlingAnalyzer("write");
this.accountName = accountName;
+ LOG.debug("Client-side throttling is enabled for the ABFS file system for
the account : " + "{}", accountName);
Review Comment:
nit: remove the + and just merge the two strings.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOperationMetrics.java:
##########
@@ -19,22 +19,29 @@
package org.apache.hadoop.fs.azurebfs.services;
import java.util.concurrent.atomic.AtomicLong;
+
/**
* Stores Abfs operation metrics during each analysis period.
*/
class AbfsOperationMetrics {
- private AtomicLong bytesFailed;
+ // No of bytes which could not be transferred due to a failed operation
Review Comment:
nit, make these javadocs, and add a "." at the end of each string to keep
javadoc tool happy.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java:
##########
@@ -242,6 +242,13 @@ private void completeExecute(TracingContext tracingContext)
private boolean executeHttpOperation(final int retryCount,
TracingContext tracingContext) throws AzureBlobFileSystemException {
AbfsHttpOperation httpOperation = null;
+ String accountName = this.client.getAbfsConfiguration().getAccountName();
Review Comment:
remove the this. ; even better, save client.getAbfsConfiguration() to a
variable and reuse
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/ConfigurationKeys.java:
##########
@@ -38,6 +38,7 @@ public final class ConfigurationKeys {
public static final String FS_AZURE_ACCOUNT_KEY_PROPERTY_NAME =
"fs.azure.account.key";
public static final String FS_AZURE_ACCOUNT_KEY_PROPERTY_NAME_REGX =
"fs\\.azure\\.account\\.key\\.(.*)";
public static final String FS_AZURE_SECURE_MODE = "fs.azure.secure.mode";
+ public static final String FS_AZURE_ACCOUNT_IS_SINGLETON_ENABLED =
"fs.azure.account.singleton.enabled";
Review Comment:
is this the right key? something about throttling would be more informative
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOperationMetrics.java:
##########
@@ -0,0 +1,74 @@
+/**
+ * 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.concurrent.atomic.AtomicLong;
+/**
+ * Stores Abfs operation metrics during each analysis period.
+ */
+class AbfsOperationMetrics {
+
+ private AtomicLong bytesFailed;
+
+ private AtomicLong bytesSuccessful;
+
+ private AtomicLong operationsFailed;
+
+ private AtomicLong operationsSuccessful;
+
+ private long endTime;
+
+ private long startTime;
+
+ AbfsOperationMetrics(long startTime) {
+ this.startTime = startTime;
+ this.bytesFailed = new AtomicLong();
+ this.bytesSuccessful = new AtomicLong();
+ this.operationsFailed = new AtomicLong();
+ this.operationsSuccessful = new AtomicLong();
+ }
+
+ AtomicLong getBytesFailed() {
+ return bytesFailed;
+ }
+
+ AtomicLong getBytesSuccessful() {
+ return bytesSuccessful;
+ }
+
+ AtomicLong getOperationsFailed() {
+ return operationsFailed;
+ }
+
+ AtomicLong getOperationsSuccessful() {
+ return operationsSuccessful;
+ }
+
+ long getEndTime() {
+ return endTime;
+ }
+
+ void setEndTime(final long endTime) {
+ this.endTime = endTime;
+ }
+
+ long getStartTime() {
+ return startTime;
+ }
+}
Review Comment:
sorry, i meant a newline *after the final }*. if you look at the github
review it is flagging the lack of a final newline as something it doesn't like
--
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]
