[
https://issues.apache.org/jira/browse/HADOOP-18457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17635027#comment-17635027
]
ASF GitHub Bot commented on HADOOP-18457:
-----------------------------------------
steveloughran commented on code in PR #5034:
URL: https://github.com/apache/hadoop/pull/5034#discussion_r1024562305
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsThrottlingInterceptFactory.java:
##########
@@ -0,0 +1,102 @@
+/**
+ * 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.ArrayList;
+import java.util.List;
+
+import org.apache.hadoop.fs.azurebfs.AbfsConfiguration;
+import org.apache.hadoop.util.WeakReferenceMap;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Class to get an instance of throttling intercept class per account.
+ */
+final class AbfsThrottlingInterceptFactory {
+
+ private AbfsThrottlingInterceptFactory() {
+ }
+
+ private static AbfsConfiguration abfsConfig;
+
+ /**
+ * List of references notified of loss.
+ */
+ private static List<String> lostReferences = new ArrayList<>();
+
+ private static final Logger LOG = LoggerFactory.getLogger(
+ AbfsThrottlingInterceptFactory.class);
+
+ /**
+ * Map which stores instance of ThrottlingIntercept class per account.
+ */
+ private static WeakReferenceMap<String, AbfsThrottlingIntercept>
+ interceptMap = new WeakReferenceMap<>(
+ AbfsThrottlingInterceptFactory::factory,
+ AbfsThrottlingInterceptFactory::referenceLost);
+
+ /**
+ * Returns instance of throttling intercept.
+ * @param accountName Account name.
+ * @return instance of throttling intercept.
+ */
+ private static AbfsClientThrottlingIntercept factory(final String
accountName) {
+ return new AbfsClientThrottlingIntercept(accountName, abfsConfig);
+ }
+
+ /**
+ * Reference lost callback.
+ * @param accountName key lost.
+ */
+ private static void referenceLost(String accountName) {
+ lostReferences.add(accountName);
+ }
+
+ /**
+ * Returns an instance of AbfsThrottlingIntercept.
+ *
+ * @param accountName The account for which we need instance of throttling
intercept.
+ @param abfsConfiguration The object of abfsconfiguration class.
+ * @return Instance of AbfsThrottlingIntercept.
+ */
+ static synchronized AbfsThrottlingIntercept getInstance(String accountName,
+ AbfsConfiguration abfsConfiguration) {
+ abfsConfig = abfsConfiguration;
+ AbfsThrottlingIntercept intercept;
+ if (!abfsConfiguration.isAutoThrottlingEnabled()) {
+ return new AbfsNoOpThrottlingIntercept();
Review Comment:
this could be made a constant, e.g have a single static final
`AbfsNoOpThrottlingIntercept.INSTANCE` they all share
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientThrottlingAnalyzer.java:
##########
@@ -54,7 +53,7 @@ class AbfsClientThrottlingAnalyzer {
private Timer timer = null;
private AtomicReference<AbfsOperationMetrics> blobMetrics = null;
private AtomicLong lastExecutionTime = null;
- private AtomicBoolean isAccountIdle = null;
+ private AtomicBoolean isOperationOnAccountIdle = null;
Review Comment:
make final and construct here or line 98
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsThrottlingIntercept.java:
##########
@@ -0,0 +1,37 @@
+/**
+ * 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 org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+
+/**
+ * An interface for Abfs Throttling Interface.
+ */
[email protected]
[email protected]
+public interface AbfsThrottlingIntercept {
+
+ void updateMetrics(AbfsRestOperationType operationType,
Review Comment:
can you add the javadocs to say what is expected here?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java:
##########
@@ -94,6 +94,9 @@ public final class FileSystemConfigurations {
public static final boolean DEFAULT_ENABLE_FLUSH = true;
public static final boolean DEFAULT_DISABLE_OUTPUTSTREAM_FLUSH = true;
public static final boolean DEFAULT_ENABLE_AUTOTHROTTLING = true;
+ public static final boolean
DEFAULT_FS_AZURE_ACCOUNT_LEVEL_THROTTLING_ENABLED = true;
+ public static final int DEFAULT_ACCOUNT_OPERATION_IDLE_TIMEOUT_MS = 60 *
1000;
+ public static final int DEFAULT_ANALYSIS_PERIOD_MS = 10 * 1000;
Review Comment:
can you use 10_000 and 60_000 for these constants; little java8 feature
which sets a good example for others
> ABFS: Support for account level throttling
> ------------------------------------------
>
> Key: HADOOP-18457
> URL: https://issues.apache.org/jira/browse/HADOOP-18457
> Project: Hadoop Common
> Issue Type: Sub-task
> Affects Versions: 3.3.4
> Reporter: Anmol Asrani
> Assignee: Anmol Asrani
> Priority: Major
> Labels: pull-request-available
> Fix For: 3.4.0
>
>
> To add support for throttling at account level
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
