[
https://issues.apache.org/jira/browse/YARN-6519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15982320#comment-15982320
]
Weiwei Yang commented on YARN-6519:
-----------------------------------
Hi [~Naganarasimha]
Thanks for the review
bq. NodeType ln no 29, no need to change the access specifier of the constructor
Intellij gives a warning {{Modifier private is redundant for enum
constructors}} if I keep the private modifier. This is because
[http://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.9.2] says
{noformat}
In an enum declaration, a constructor declaration with no access modifiers is
private.
{noformat}
bq. EMPTY_CONTAINER_LIST can be private
Sure, fixed.
bq. QueueMetrics ln no 151, i was of the opinion that we use naming convention
of field as its not exactly a string constant, thoughts?
This is using name convention for {{final}} and {{static}} field, if not name
it this way, we will get checkstyle warnings, similar names can be found a lot
of places, such as
{noformat}
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/erasurecode/CodecUtil.java:98:
public static final Map<String, String> DEFAULT_CODERS_MAP = ImmutableMap.of(
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/nativeio/NativeIO.java:465:
private static final Map<Integer, CachedName> USER_ID_NAME_CACHE =
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/nativeio/NativeIO.java:468:
private static final Map<Integer, CachedName> GROUP_ID_NAME_CACHE =
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/nativeio/NativeIO.java:702:
private static final Map<Long, CachedUid> uidCache =
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/ObjectWritable.java:85:
private static final Map<String, Class<?>> PRIMITIVE_NAMES = new
HashMap<String, Class<?>>();
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/WritableFactories.java:33:
private static final Map<Class, WritableFactory> CLASS_TO_FACTORY =
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:190:
private static final Map<Class<?>,RpcEngine> PROTOCOL_ENGINES
{noformat}
4. is it possible for concurrent modification exception ?
Good catch! Thanks! Fixed with following
{code}
preemptionCandidates.entrySet()
.removeIf(candidate ->
candidate.getValue() + 2 * maxWaitTime < currentTime);
{code}
5. FSSchedulerNode ln no 157-165, i presume for removal we better not use entry
set,
Also a good catch, thanks. In order to fix the findbugs warning, we can still
use entry set, just to iterate over this entry set
{code}
Iterator<Map.Entry<FSAppAttempt, Resource>> iterator =
resourcesPreemptedForApp.entrySet().iterator();
{code}
Thanks for all the comments! Appreciate your review.
> Fix warnings from Spotbugs in hadoop-yarn-server-resourcemanager
> ----------------------------------------------------------------
>
> Key: YARN-6519
> URL: https://issues.apache.org/jira/browse/YARN-6519
> Project: Hadoop YARN
> Issue Type: Bug
> Components: resourcemanager
> Reporter: Weiwei Yang
> Assignee: Weiwei Yang
> Labels: findbugs
> Attachments: YARN-6519.001.patch
>
>
> There is 8 findbugs warning in hadoop-yarn-server-timelineservice since
> switched to spotbugs
> #
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerQueueManager$1.compare(CSQueue,
> CSQueue) incorrectly handles float value
> # org.apache.hadoop.yarn.server.resourcemanager.scheduler.NodeType.index
> field is public and mutable
> #
> org.apache.hadoop.yarn.server.resourcemanager.ApplicationMasterService.EMPTY_CONTAINER_LIST
> is a mutable collection which should be package protected
> #
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.AbstractYarnScheduler.EMPTY_CONTAINER_LIST
> is a mutable collection which should be package protected
> #
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.QueueMetrics.queueMetrics
> is a mutable collection
> #
> org.apache.hadoop.yarn.server.resourcemanager.monitor.capacity.ProportionalCapacityPreemptionPolicy.cleanupStaledPreemptionCandidates(long)
> makes inefficient use of keySet iterator instead of entrySet iterator
> #
> org.apache.hadoop.yarn.server.resourcemanager.rmapp.attempt.RMAppAttemptImpl.transferStateFromAttempt(RMAppAttempt)
> makes inefficient use of keySet iterator instead of entrySet iterator
> #
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FSSchedulerNode.cleanupPreemptionList()
> makes inefficient use of keySet iterator instead of entrySet iterator
> See more from
> [https://builds.apache.org/job/PreCommit-HADOOP-Build/12157/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-warnings.html]
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
