Skip to content

Commit

Permalink
YARN-2938. Fixed new findbugs warnings in hadoop-yarn-resourcemanager…
Browse files Browse the repository at this point in the history 
… and hadoop-yarn-applicationhistoryservice. Contributed by Varun Saxena.
  • Loading branch information
@zjshen14
zjshen14 committed Dec 29, 2014
1 parent 1454efe commit 241d3b3
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 42 deletions.
3 changes: 3 additions & 0 deletions hadoop-yarn-project/CHANGES.txt
View file
Expand Up @@ -292,6 +292,9 @@ Release 2.7.0 - UNRELEASED
YARN-2992. ZKRMStateStore crashes due to session expiry. (Karthik Kambatla YARN-2992. ZKRMStateStore crashes due to session expiry. (Karthik Kambatla
via jianhe) via jianhe)


YARN-2938. Fixed new findbugs warnings in hadoop-yarn-resourcemanager and
hadoop-yarn-applicationhistoryservice. (Varun Saxena via zjshen)

Release 2.6.0 - 2014-11-18 Release 2.6.0 - 2014-11-18


INCOMPATIBLE CHANGES INCOMPATIBLE CHANGES
Expand Down
32 changes: 32 additions & 0 deletions hadoop-yarn-project/hadoop-yarn/dev-support/findbugs-exclude.xml
View file
Expand Up @@ -417,4 +417,36 @@
<Class name="org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.sharedcache.SharedCacheUploadService" /> <Class name="org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.sharedcache.SharedCacheUploadService" />
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" /> <Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
</Match> </Match>

<Match>
<Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerApplicationAttempt" />
<Field name="queue" />
<Bug pattern="IS2_INCONSISTENT_SYNC" />
</Match>
<Match>
<Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler" />
<Field name="allocConf" />
<Bug pattern="IS2_INCONSISTENT_SYNC" />
</Match>
<Match>
<Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode" />
<Field name="numContainers" />
<Bug pattern="VO_VOLATILE_INCREMENT" />
</Match>
<Match>
<Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.AbstractCSQueue" />
<Field name="numContainers" />
<Bug pattern="VO_VOLATILE_INCREMENT" />
</Match>
<Match>
<Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.ParentQueue" />
<Field name="numApplications" />
<Bug pattern="VO_VOLATILE_INCREMENT" />
</Match>
<Match>
<Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.AbstractYarnScheduler" />
<Method name="recoverContainersOnNode" />
<Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" />
</Match>

</FindBugsFilter> </FindBugsFilter>
73 changes: 39 additions & 34 deletions ...oryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/LeveldbTimelineStore.java
View file
Expand Up @@ -48,6 +48,7 @@


import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.nio.charset.Charset;
import java.util.*; import java.util.*;
import java.util.Map.Entry; import java.util.Map.Entry;
import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantLock;
Expand Down Expand Up @@ -120,25 +121,25 @@ public class LeveldbTimelineStore extends AbstractService
@VisibleForTesting @VisibleForTesting
static final String FILENAME = "leveldb-timeline-store.ldb"; static final String FILENAME = "leveldb-timeline-store.ldb";


private static final byte[] START_TIME_LOOKUP_PREFIX = "k".getBytes(); private static final byte[] START_TIME_LOOKUP_PREFIX = "k".getBytes(Charset.forName("UTF-8"));
private static final byte[] ENTITY_ENTRY_PREFIX = "e".getBytes(); private static final byte[] ENTITY_ENTRY_PREFIX = "e".getBytes(Charset.forName("UTF-8"));
private static final byte[] INDEXED_ENTRY_PREFIX = "i".getBytes(); private static final byte[] INDEXED_ENTRY_PREFIX = "i".getBytes(Charset.forName("UTF-8"));


private static final byte[] EVENTS_COLUMN = "e".getBytes(); private static final byte[] EVENTS_COLUMN = "e".getBytes(Charset.forName("UTF-8"));
private static final byte[] PRIMARY_FILTERS_COLUMN = "f".getBytes(); private static final byte[] PRIMARY_FILTERS_COLUMN = "f".getBytes(Charset.forName("UTF-8"));
private static final byte[] OTHER_INFO_COLUMN = "i".getBytes(); private static final byte[] OTHER_INFO_COLUMN = "i".getBytes(Charset.forName("UTF-8"));
private static final byte[] RELATED_ENTITIES_COLUMN = "r".getBytes(); private static final byte[] RELATED_ENTITIES_COLUMN = "r".getBytes(Charset.forName("UTF-8"));
private static final byte[] INVISIBLE_REVERSE_RELATED_ENTITIES_COLUMN = private static final byte[] INVISIBLE_REVERSE_RELATED_ENTITIES_COLUMN =
"z".getBytes(); "z".getBytes(Charset.forName("UTF-8"));
private static final byte[] DOMAIN_ID_COLUMN = "d".getBytes(); private static final byte[] DOMAIN_ID_COLUMN = "d".getBytes(Charset.forName("UTF-8"));


private static final byte[] DOMAIN_ENTRY_PREFIX = "d".getBytes(); private static final byte[] DOMAIN_ENTRY_PREFIX = "d".getBytes(Charset.forName("UTF-8"));
private static final byte[] OWNER_LOOKUP_PREFIX = "o".getBytes(); private static final byte[] OWNER_LOOKUP_PREFIX = "o".getBytes(Charset.forName("UTF-8"));
private static final byte[] DESCRIPTION_COLUMN = "d".getBytes(); private static final byte[] DESCRIPTION_COLUMN = "d".getBytes(Charset.forName("UTF-8"));
private static final byte[] OWNER_COLUMN = "o".getBytes(); private static final byte[] OWNER_COLUMN = "o".getBytes(Charset.forName("UTF-8"));
private static final byte[] READER_COLUMN = "r".getBytes(); private static final byte[] READER_COLUMN = "r".getBytes(Charset.forName("UTF-8"));
private static final byte[] WRITER_COLUMN = "w".getBytes(); private static final byte[] WRITER_COLUMN = "w".getBytes(Charset.forName("UTF-8"));
private static final byte[] TIMESTAMP_COLUMN = "t".getBytes(); private static final byte[] TIMESTAMP_COLUMN = "t".getBytes(Charset.forName("UTF-8"));


private static final byte[] EMPTY_BYTES = new byte[0]; private static final byte[] EMPTY_BYTES = new byte[0];


Expand Down Expand Up @@ -440,7 +441,7 @@ private static TimelineEntity getEntity(String entityId, String entityType,
} }
} else if (key[prefixlen] == DOMAIN_ID_COLUMN[0]) { } else if (key[prefixlen] == DOMAIN_ID_COLUMN[0]) {
byte[] v = iterator.peekNext().getValue(); byte[] v = iterator.peekNext().getValue();
String domainId = new String(v); String domainId = new String(v, Charset.forName("UTF-8"));
entity.setDomainId(domainId); entity.setDomainId(domainId);
} else { } else {
if (key[prefixlen] != if (key[prefixlen] !=
Expand Down Expand Up @@ -790,7 +791,7 @@ private void put(TimelineEntity entity, TimelinePutResponse response,
if (domainIdBytes == null) { if (domainIdBytes == null) {
domainId = TimelineDataManager.DEFAULT_DOMAIN_ID; domainId = TimelineDataManager.DEFAULT_DOMAIN_ID;
} else { } else {
domainId = new String(domainIdBytes); domainId = new String(domainIdBytes, Charset.forName("UTF-8"));
} }
if (!domainId.equals(entity.getDomainId())) { if (!domainId.equals(entity.getDomainId())) {
// in this case the entity will be put, but the relation will be // in this case the entity will be put, but the relation will be
Expand Down Expand Up @@ -845,9 +846,9 @@ private void put(TimelineEntity entity, TimelinePutResponse response,
return; return;
} }
} else { } else {
writeBatch.put(key, entity.getDomainId().getBytes()); writeBatch.put(key, entity.getDomainId().getBytes(Charset.forName("UTF-8")));
writePrimaryFilterEntries(writeBatch, primaryFilters, key, writePrimaryFilterEntries(writeBatch, primaryFilters, key,
entity.getDomainId().getBytes()); entity.getDomainId().getBytes(Charset.forName("UTF-8")));
} }
db.write(writeBatch); db.write(writeBatch);
} catch (DBException de) { } catch (DBException de) {
Expand Down Expand Up @@ -879,7 +880,7 @@ private void put(TimelineEntity entity, TimelinePutResponse response,
// This is the new entity, the domain should be the same // This is the new entity, the domain should be the same
byte[] key = createDomainIdKey(relatedEntity.getId(), byte[] key = createDomainIdKey(relatedEntity.getId(),
relatedEntity.getType(), relatedEntityStartTime); relatedEntity.getType(), relatedEntityStartTime);
db.put(key, entity.getDomainId().getBytes()); db.put(key, entity.getDomainId().getBytes(Charset.forName("UTF-8")));
db.put(createRelatedEntityKey(relatedEntity.getId(), db.put(createRelatedEntityKey(relatedEntity.getId(),
relatedEntity.getType(), relatedEntityStartTime, relatedEntity.getType(), relatedEntityStartTime,
entity.getEntityId(), entity.getEntityType()), EMPTY_BYTES); entity.getEntityId(), entity.getEntityType()), EMPTY_BYTES);
Expand Down Expand Up @@ -1206,7 +1207,7 @@ private static byte[] createOtherInfoKey(String entityId, String entityType,
* to the end of the array (for parsing other info keys). * to the end of the array (for parsing other info keys).
*/ */
private static String parseRemainingKey(byte[] b, int offset) { private static String parseRemainingKey(byte[] b, int offset) {
return new String(b, offset, b.length - offset); return new String(b, offset, b.length - offset, Charset.forName("UTF-8"));
} }


/** /**
Expand Down Expand Up @@ -1589,8 +1590,10 @@ public void put(TimelineDomain domain) throws IOException {
byte[] ownerLookupEntryKey = createOwnerLookupKey( byte[] ownerLookupEntryKey = createOwnerLookupKey(
domain.getOwner(), domain.getId(), DESCRIPTION_COLUMN); domain.getOwner(), domain.getId(), DESCRIPTION_COLUMN);
if (domain.getDescription() != null) { if (domain.getDescription() != null) {
writeBatch.put(domainEntryKey, domain.getDescription().getBytes()); writeBatch.put(domainEntryKey, domain.getDescription().
writeBatch.put(ownerLookupEntryKey, domain.getDescription().getBytes()); getBytes(Charset.forName("UTF-8")));
writeBatch.put(ownerLookupEntryKey, domain.getDescription().
getBytes(Charset.forName("UTF-8")));
} else { } else {
writeBatch.put(domainEntryKey, EMPTY_BYTES); writeBatch.put(domainEntryKey, EMPTY_BYTES);
writeBatch.put(ownerLookupEntryKey, EMPTY_BYTES); writeBatch.put(ownerLookupEntryKey, EMPTY_BYTES);
Expand All @@ -1601,16 +1604,17 @@ public void put(TimelineDomain domain) throws IOException {
ownerLookupEntryKey = createOwnerLookupKey( ownerLookupEntryKey = createOwnerLookupKey(
domain.getOwner(), domain.getId(), OWNER_COLUMN); domain.getOwner(), domain.getId(), OWNER_COLUMN);
// Null check for owner is done before // Null check for owner is done before
writeBatch.put(domainEntryKey, domain.getOwner().getBytes()); writeBatch.put(domainEntryKey, domain.getOwner().getBytes(Charset.forName("UTF-8")));
writeBatch.put(ownerLookupEntryKey, domain.getOwner().getBytes()); writeBatch.put(ownerLookupEntryKey, domain.getOwner().getBytes(Charset.forName("UTF-8")));


// Write readers // Write readers
domainEntryKey = createDomainEntryKey(domain.getId(), READER_COLUMN); domainEntryKey = createDomainEntryKey(domain.getId(), READER_COLUMN);
ownerLookupEntryKey = createOwnerLookupKey( ownerLookupEntryKey = createOwnerLookupKey(
domain.getOwner(), domain.getId(), READER_COLUMN); domain.getOwner(), domain.getId(), READER_COLUMN);
if (domain.getReaders() != null && domain.getReaders().length() > 0) { if (domain.getReaders() != null && domain.getReaders().length() > 0) {
writeBatch.put(domainEntryKey, domain.getReaders().getBytes()); writeBatch.put(domainEntryKey, domain.getReaders().getBytes(Charset.forName("UTF-8")));
writeBatch.put(ownerLookupEntryKey, domain.getReaders().getBytes()); writeBatch.put(ownerLookupEntryKey, domain.getReaders().
getBytes(Charset.forName("UTF-8")));
} else { } else {
writeBatch.put(domainEntryKey, EMPTY_BYTES); writeBatch.put(domainEntryKey, EMPTY_BYTES);
writeBatch.put(ownerLookupEntryKey, EMPTY_BYTES); writeBatch.put(ownerLookupEntryKey, EMPTY_BYTES);
Expand All @@ -1621,8 +1625,9 @@ public void put(TimelineDomain domain) throws IOException {
ownerLookupEntryKey = createOwnerLookupKey( ownerLookupEntryKey = createOwnerLookupKey(
domain.getOwner(), domain.getId(), WRITER_COLUMN); domain.getOwner(), domain.getId(), WRITER_COLUMN);
if (domain.getWriters() != null && domain.getWriters().length() > 0) { if (domain.getWriters() != null && domain.getWriters().length() > 0) {
writeBatch.put(domainEntryKey, domain.getWriters().getBytes()); writeBatch.put(domainEntryKey, domain.getWriters().getBytes(Charset.forName("UTF-8")));
writeBatch.put(ownerLookupEntryKey, domain.getWriters().getBytes()); writeBatch.put(ownerLookupEntryKey, domain.getWriters().
getBytes(Charset.forName("UTF-8")));
} else { } else {
writeBatch.put(domainEntryKey, EMPTY_BYTES); writeBatch.put(domainEntryKey, EMPTY_BYTES);
writeBatch.put(ownerLookupEntryKey, EMPTY_BYTES); writeBatch.put(ownerLookupEntryKey, EMPTY_BYTES);
Expand Down Expand Up @@ -1759,13 +1764,13 @@ private static TimelineDomain getTimelineDomain(
byte[] value = iterator.peekNext().getValue(); byte[] value = iterator.peekNext().getValue();
if (value != null && value.length > 0) { if (value != null && value.length > 0) {
if (key[prefix.length] == DESCRIPTION_COLUMN[0]) { if (key[prefix.length] == DESCRIPTION_COLUMN[0]) {
domain.setDescription(new String(value)); domain.setDescription(new String(value, Charset.forName("UTF-8")));
} else if (key[prefix.length] == OWNER_COLUMN[0]) { } else if (key[prefix.length] == OWNER_COLUMN[0]) {
domain.setOwner(new String(value)); domain.setOwner(new String(value, Charset.forName("UTF-8")));
} else if (key[prefix.length] == READER_COLUMN[0]) { } else if (key[prefix.length] == READER_COLUMN[0]) {
domain.setReaders(new String(value)); domain.setReaders(new String(value, Charset.forName("UTF-8")));
} else if (key[prefix.length] == WRITER_COLUMN[0]) { } else if (key[prefix.length] == WRITER_COLUMN[0]) {
domain.setWriters(new String(value)); domain.setWriters(new String(value, Charset.forName("UTF-8")));
} else if (key[prefix.length] == TIMESTAMP_COLUMN[0]) { } else if (key[prefix.length] == TIMESTAMP_COLUMN[0]) {
domain.setCreatedTime(readReverseOrderedLong(value, 0)); domain.setCreatedTime(readReverseOrderedLong(value, 0));
domain.setModifiedTime(readReverseOrderedLong(value, 8)); domain.setModifiedTime(readReverseOrderedLong(value, 8));
Expand Down
2 changes: 1 addition & 1 deletion ...toryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/MemoryTimelineStore.java
View file
Expand Up @@ -398,7 +398,7 @@ public synchronized TimelinePutResponse put(TimelineEntities data) {
public void put(TimelineDomain domain) throws IOException { public void put(TimelineDomain domain) throws IOException {
TimelineDomain domainToReplace = TimelineDomain domainToReplace =
domainsById.get(domain.getId()); domainsById.get(domain.getId());
long currentTimestamp = System.currentTimeMillis(); Long currentTimestamp = System.currentTimeMillis();
TimelineDomain domainToStore = createTimelineDomain( TimelineDomain domainToStore = createTimelineDomain(
domain.getId(), domain.getDescription(), domain.getOwner(), domain.getId(), domain.getDescription(), domain.getOwner(),
domain.getReaders(), domain.getWriters(), domain.getReaders(), domain.getWriters(),
Expand Down
9 changes: 7 additions & 2 deletions .../apache/hadoop/yarn/server/timeline/security/TimelineAuthenticationFilterInitializer.java
View file
Expand Up @@ -18,9 +18,12 @@


package org.apache.hadoop.yarn.server.timeline.security; package org.apache.hadoop.yarn.server.timeline.security;


import java.io.FileReader; import java.io.File;
import java.io.FileInputStream;
import java.io.IOException; import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader; import java.io.Reader;
import java.nio.charset.Charset;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;


Expand Down Expand Up @@ -113,7 +116,9 @@ public void initFilter(FilterContainer container, Configuration conf) {
Reader reader = null; Reader reader = null;
try { try {
StringBuilder secret = new StringBuilder(); StringBuilder secret = new StringBuilder();
reader = new FileReader(signatureSecretFile); reader = new InputStreamReader(new FileInputStream(new File(signatureSecretFile)),
Charset.forName("UTF-8"));

int c = reader.read(); int c = reader.read();
while (c > -1) { while (c > -1) {
secret.append((char) c); secret.append((char) c);
Expand Down
3 changes: 2 additions & 1 deletion .../src/main/java/org/apache/hadoop/yarn/server/resourcemanager/recovery/ZKRMStateStore.java
View file
Expand Up @@ -23,6 +23,7 @@
import java.io.DataInputStream; import java.io.DataInputStream;
import java.io.DataOutputStream; import java.io.DataOutputStream;
import java.io.IOException; import java.io.IOException;
import java.nio.charset.Charset;
import java.security.NoSuchAlgorithmException; import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom; import java.security.SecureRandom;
import java.util.ArrayList; import java.util.ArrayList;
Expand Down Expand Up @@ -1108,7 +1109,7 @@ private synchronized void createConnection()
} }
if (useDefaultFencingScheme) { if (useDefaultFencingScheme) {
zkClient.addAuthInfo(zkRootNodeAuthScheme, zkClient.addAuthInfo(zkRootNodeAuthScheme,
(zkRootNodeUsername + ":" + zkRootNodePassword).getBytes()); (zkRootNodeUsername + ":" + zkRootNodePassword).getBytes(Charset.forName("UTF-8")));
} }
} catch (IOException ioe) { } catch (IOException ioe) {
// Retry in case of network failures // Retry in case of network failures
Expand Down
8 changes: 8 additions & 0 deletions ...anager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmnode/RMNodeImpl.java
View file
Expand Up @@ -448,6 +448,8 @@ private void updateMetricsForRejoinedNode(NodeState previousNodeState) {
case UNHEALTHY: case UNHEALTHY:
metrics.decrNumUnhealthyNMs(); metrics.decrNumUnhealthyNMs();
break; break;
default:
LOG.debug("Unexpected previous node state");
} }
} }


Expand All @@ -462,6 +464,8 @@ private void updateMetricsForDeactivatedNode(NodeState initialState,
case UNHEALTHY: case UNHEALTHY:
metrics.decrNumUnhealthyNMs(); metrics.decrNumUnhealthyNMs();
break; break;
default:
LOG.debug("Unexpected inital state");
} }


switch (finalState) { switch (finalState) {
Expand All @@ -477,6 +481,8 @@ private void updateMetricsForDeactivatedNode(NodeState initialState,
case UNHEALTHY: case UNHEALTHY:
metrics.incrNumUnhealthyNMs(); metrics.incrNumUnhealthyNMs();
break; break;
default:
LOG.debug("Unexpected final state");
} }
} }


Expand Down Expand Up @@ -583,6 +589,8 @@ public void transition(RMNodeImpl rmNode, RMNodeEvent event) {
case UNHEALTHY: case UNHEALTHY:
ClusterMetrics.getMetrics().decrNumUnhealthyNMs(); ClusterMetrics.getMetrics().decrNumUnhealthyNMs();
break; break;
default:
LOG.debug("Unexpected Rmnode state");
} }
rmNode.context.getRMNodes().put(newNode.getNodeID(), newNode); rmNode.context.getRMNodes().put(newNode.getNodeID(), newNode);
rmNode.context.getDispatcher().getEventHandler().handle( rmNode.context.getDispatcher().getEventHandler().handle(
Expand Down
8 changes: 4 additions & 4 deletions ...n/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/SchedulingPolicy.java
View file
Expand Up @@ -48,10 +48,10 @@ public abstract class SchedulingPolicy {
* Returns a {@link SchedulingPolicy} instance corresponding to the passed clazz * Returns a {@link SchedulingPolicy} instance corresponding to the passed clazz
*/ */
public static SchedulingPolicy getInstance(Class<? extends SchedulingPolicy> clazz) { public static SchedulingPolicy getInstance(Class<? extends SchedulingPolicy> clazz) {
SchedulingPolicy policy = instances.get(clazz); SchedulingPolicy policy = ReflectionUtils.newInstance(clazz, null);
if (policy == null) { SchedulingPolicy policyRet = instances.putIfAbsent(clazz, policy);
policy = ReflectionUtils.newInstance(clazz, null); if(policyRet != null) {
instances.put(clazz, policy); return policyRet;
} }
return policy; return policy;
} }
Expand Down
2 changes: 2 additions & 0 deletions ...manager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/NodesPage.java
View file
Expand Up @@ -92,6 +92,8 @@ protected void render(Block html) {
rmNodes = this.rm.getRMContext().getInactiveRMNodes().values(); rmNodes = this.rm.getRMContext().getInactiveRMNodes().values();
isInactive = true; isInactive = true;
break; break;
default:
LOG.debug("Unexpected state filter for inactive RM node");
} }
} }
for (RMNode ni : rmNodes) { for (RMNode ni : rmNodes) {
Expand Down

0 comments on commit 241d3b3

Please sign in to comment.