hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Weiwei Yang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-6519) Fix warnings from Spotbugs in hadoop-yarn-server-resourcemanager
Date Tue, 25 Apr 2017 03:33:04 GMT

    [ 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: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message