[ https://issues.apache.org/jira/browse/HADOOP-16237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16810757#comment-16810757 ]
Gabor Bota edited comment on HADOOP-16237 at 4/5/19 1:24 PM: ------------------------------------------------------------- h2. branch-findbugs-hadoop-common-project_hadoop-kms-warnings.html *Null passed for non-null parameter of com.google.common.base.Strings.isNullOrEmpty(String) in org.apache.hadoop.crypto.key.kms.server.KMSAudit.op(KMSAuditLogger$OpStatus, Object, UserGroupInformation, String, String, String)* The called method signature is {{public static boolean isNullOrEmpty(@Nullable String string)}} in guava 27, so this should be ignored h2. branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html *Null passed for non-null parameter of com.google.common.base.Strings.emptyToNull(String) in org.apache.hadoop.yarn.server.nodemanager.NodeHealthCheckerService.getHealthReport()* The called method signature is {{public static boolean isNullOrEmpty(@Nullable String string)}} in guava 27, so this should be ignored h2. branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-warnings.html *Null passed for non-null parameter of com.google.common.util.concurrent.SettableFuture.set(Object) in org.apache.hadoop.yarn.server.resourcemanager.recovery.RMStateStore$UpdateAppTransition.transition(RMStateStore, RMStateStoreEvent)* The called method signature is {{public boolean set(@Nullable V value)}} in guava 27, so this should be ignored *Null passed for non-null parameter of com.google.common.util.concurrent.SettableFuture.set(Object) in org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacityScheduler.updateApplicationPriority(Priority, ApplicationId, SettableFuture, UserGroupInformation)* The called method signature is {{public boolean set(@Nullable V value)}} in guava 27, so this should be ignored h2. branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-timelineservice-documentstore-warnings.html *Possible doublecheck on org.apache.hadoop.yarn.server.timelineservice.documentstore.reader.cosmosdb.CosmosDBDocumentStoreReader.client in new org.apache.hadoop.yarn.server.timelineservice.documentstore.reader.cosmosdb.CosmosDBDocumentStoreReader(Configuration)* Double-checked locking: we may want to change this to a static initializer, or leave it as is. {code:java} if (client == null) { synchronized (this) { if (client == null) { LOG.info("Creating Cosmos DB Client..."); client = DocumentStoreUtils.createCosmosDBClient(conf); } } {code} *Possible doublecheck on org.apache.hadoop.yarn.server.timelineservice.documentstore.writer.cosmosdb.CosmosDBDocumentStoreWriter.client in new org.apache.hadoop.yarn.server.timelineservice.documentstore.writer.cosmosdb.CosmosDBDocumentStoreWriter(Configuration)* Double-checked locking: we may want to change this to a static initializer, or leave it as is. {code:java} if (client == null) { synchronized (this) { if (client == null) { LOG.info("Creating Cosmos DB Client..."); client = DocumentStoreUtils.createCosmosDBClient(conf); } } } {code} *Unread field: org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.entity.TimelineEventSubDoc.valid* This field is not in use and it could be removed, although we should keep it because it seems like it's needed in a document model. *Unread field: org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.entity.TimelineMetricSubDoc.valid* This field is not in use and it could be removed, although we should keep it because it seems like it's needed in a document model. *org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.entity.TimelineEntityDocument.setEvents(Map) makes inefficient use of keySet iterator instead of entrySet iterator* Justification is not correct to change the code, because we will use the key from the keyset we iterate through: {code:java} public void setEvents(Map<String, Set<TimelineEventSubDoc>> events) { for (String eventId : events.keySet()) { for(TimelineEventSubDoc eventSubDoc: events.get(eventId)) { timelineEntity.addEvent(eventSubDoc.fetchTimelineEvent()); } if (this.events.containsKey(eventId)) { this.events.get(eventId).addAll(events.get(eventId)); } else { this.events.put(eventId, new HashSet<>(events.get(eventId))); } } } {code} so this can be ignored, although I will check if it's better to iterate throgh the entryset. *org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.entity.TimelineEntityDocument.setMetrics(Map) makes inefficient use of keySet iterator instead of entrySet iterator* Justification is not correct to change the code, because we will use the key from the keyset we iterate through: {code:java} public void setMetrics(Map<String, Set<TimelineMetricSubDoc>> metrics) { for (String metricId : metrics.keySet()) { for(TimelineMetricSubDoc metricSubDoc : metrics.get(metricId)) { timelineEntity.addMetric(metricSubDoc.fetchTimelineMetric()); } if (this.metrics.containsKey(metricId)) { this.metrics.get(metricId).addAll(metrics.get(metricId)); } else { this.metrics.put(metricId, new HashSet<>(metrics.get(metricId))); } } } {code} so this can be ignored, although I will check if it's better to iterate throgh the entryset. *org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.flowrun.FlowRunDocument.aggregateMetrics(Map) makes inefficient use of keySet iterator instead of entrySet iterator* Justification is not correct to change the code, because we will use the key from the keyset we iterate through: {code:java} private void aggregateMetrics( Map<String, TimelineMetricSubDoc> metricSubDocMap) { for(String metricId : metricSubDocMap.keySet()) { if (this.metrics.containsKey(metricId)) { TimelineMetric incomingMetric = metricSubDocMap.get(metricId).fetchTimelineMetric(); TimelineMetric baseMetric = this.metrics.get(metricId).fetchTimelineMetric(); if (incomingMetric.getValues().size() > 0) { baseMetric = aggregate(incomingMetric, baseMetric); this.metrics.put(metricId, new TimelineMetricSubDoc(baseMetric)); } else { LOG.debug("No incoming metric to aggregate for : {}", baseMetric.getId()); } } else { this.metrics.put(metricId, metricSubDocMap.get(metricId)); } } } {code} so this can be ignored, although I will check if it's better to iterate throgh the entryset. *Switch statement found in org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.flowrun.FlowRunDocument.aggregate(TimelineMetric, TimelineMetric) where default case is missing* We should add at least a log to the default - LOG.warn("Unknown TimelineMetricOperation.") was (Author: gabor.bota): h2. branch-findbugs-hadoop-common-project_hadoop-kms-warnings.html *Null passed for non-null parameter of com.google.common.base.Strings.isNullOrEmpty(String) in org.apache.hadoop.crypto.key.kms.server.KMSAudit.op(KMSAuditLogger$OpStatus, Object, UserGroupInformation, String, String, String)* The called method signature is {{public static boolean isNullOrEmpty(@Nullable String string)}} in guava 27, so this should be ignored h2. branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html *Null passed for non-null parameter of com.google.common.base.Strings.emptyToNull(String) in org.apache.hadoop.yarn.server.nodemanager.NodeHealthCheckerService.getHealthReport()* The called method signature is {{public static boolean isNullOrEmpty(@Nullable String string)}} in guava 27, so this should be ignored h2. branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-warnings.html *Null passed for non-null parameter of com.google.common.util.concurrent.SettableFuture.set(Object) in org.apache.hadoop.yarn.server.resourcemanager.recovery.RMStateStore$UpdateAppTransition.transition(RMStateStore, RMStateStoreEvent)* The called method signature is {{public boolean set(@Nullable V value)}} in guava 27, so this should be ignored *Null passed for non-null parameter of com.google.common.util.concurrent.SettableFuture.set(Object) in org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacityScheduler.updateApplicationPriority(Priority, ApplicationId, SettableFuture, UserGroupInformation)* The called method signature is {{public boolean set(@Nullable V value)}} in guava 27, so this should be ignored h2. branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-timelineservice-documentstore-warnings.html *Possible doublecheck on org.apache.hadoop.yarn.server.timelineservice.documentstore.reader.cosmosdb.CosmosDBDocumentStoreReader.client in new org.apache.hadoop.yarn.server.timelineservice.documentstore.reader.cosmosdb.CosmosDBDocumentStoreReader(Configuration)* We need this for nullcheck before and after going to synchronized, do this should be ignored. {code:java} if (client == null) { synchronized (this) { if (client == null) { LOG.info("Creating Cosmos DB Client..."); client = DocumentStoreUtils.createCosmosDBClient(conf); } } {code} *Possible doublecheck on org.apache.hadoop.yarn.server.timelineservice.documentstore.writer.cosmosdb.CosmosDBDocumentStoreWriter.client in new org.apache.hadoop.yarn.server.timelineservice.documentstore.writer.cosmosdb.CosmosDBDocumentStoreWriter(Configuration)* We need this for nullcheck before and after going to synchronized, do this should be ignored. {code:java} if (client == null) { synchronized (this) { if (client == null) { LOG.info("Creating Cosmos DB Client..."); client = DocumentStoreUtils.createCosmosDBClient(conf); } } } {code} *Unread field: org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.entity.TimelineEventSubDoc.valid* This field is not in use and it could be removed, although we should keep it because it seems like it's needed in a document model. *Unread field: org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.entity.TimelineMetricSubDoc.valid* This field is not in use and it could be removed, although we should keep it because it seems like it's needed in a document model. *org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.entity.TimelineEntityDocument.setEvents(Map) makes inefficient use of keySet iterator instead of entrySet iterator* Justification is not correct to change the code, because we will use the key from the keyset we iterate through: {code:java} public void setEvents(Map<String, Set<TimelineEventSubDoc>> events) { for (String eventId : events.keySet()) { for(TimelineEventSubDoc eventSubDoc: events.get(eventId)) { timelineEntity.addEvent(eventSubDoc.fetchTimelineEvent()); } if (this.events.containsKey(eventId)) { this.events.get(eventId).addAll(events.get(eventId)); } else { this.events.put(eventId, new HashSet<>(events.get(eventId))); } } } {code} so this can be ignored, although I will check if it's better to iterate throgh the entryset. *org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.entity.TimelineEntityDocument.setMetrics(Map) makes inefficient use of keySet iterator instead of entrySet iterator* Justification is not correct to change the code, because we will use the key from the keyset we iterate through: {code:java} public void setMetrics(Map<String, Set<TimelineMetricSubDoc>> metrics) { for (String metricId : metrics.keySet()) { for(TimelineMetricSubDoc metricSubDoc : metrics.get(metricId)) { timelineEntity.addMetric(metricSubDoc.fetchTimelineMetric()); } if (this.metrics.containsKey(metricId)) { this.metrics.get(metricId).addAll(metrics.get(metricId)); } else { this.metrics.put(metricId, new HashSet<>(metrics.get(metricId))); } } } {code} so this can be ignored, although I will check if it's better to iterate throgh the entryset. *org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.flowrun.FlowRunDocument.aggregateMetrics(Map) makes inefficient use of keySet iterator instead of entrySet iterator* Justification is not correct to change the code, because we will use the key from the keyset we iterate through: {code:java} private void aggregateMetrics( Map<String, TimelineMetricSubDoc> metricSubDocMap) { for(String metricId : metricSubDocMap.keySet()) { if (this.metrics.containsKey(metricId)) { TimelineMetric incomingMetric = metricSubDocMap.get(metricId).fetchTimelineMetric(); TimelineMetric baseMetric = this.metrics.get(metricId).fetchTimelineMetric(); if (incomingMetric.getValues().size() > 0) { baseMetric = aggregate(incomingMetric, baseMetric); this.metrics.put(metricId, new TimelineMetricSubDoc(baseMetric)); } else { LOG.debug("No incoming metric to aggregate for : {}", baseMetric.getId()); } } else { this.metrics.put(metricId, metricSubDocMap.get(metricId)); } } } {code} so this can be ignored, although I will check if it's better to iterate throgh the entryset. *Switch statement found in org.apache.hadoop.yarn.server.timelineservice.documentstore.collection.document.flowrun.FlowRunDocument.aggregate(TimelineMetric, TimelineMetric) where default case is missing* We should add at least a log to the default - LOG.warn("Unknown TimelineMetricOperation.") > Fix new findbugs issues after update guava to 27.0-jre in hadoop-project trunk > ------------------------------------------------------------------------------ > > Key: HADOOP-16237 > URL: https://issues.apache.org/jira/browse/HADOOP-16237 > Project: Hadoop Common > Issue Type: Sub-task > Affects Versions: 3.3.0 > Reporter: Gabor Bota > Assignee: Gabor Bota > Priority: Critical > Attachments: > branch-findbugs-hadoop-common-project_hadoop-kms-warnings.html, > branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html, > > branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-warnings.html, > > branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-timelineservice-documentstore-warnings.html > > > There are a bunch of new findbugs issues in the build after committing the > guava update. > Mostly in yarn, but we have to check and handle those. -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org