[
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 5:40 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 {{@Nullable String emptyToNull(@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
add volatile to client, 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
add volatile to client, 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 {{@Nullable String emptyToNull(@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
add volatile to client, 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
add volatile to client, 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.")
> 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: [email protected]
For additional commands, e-mail: [email protected]
