[HADOOP-14807] should prevent the possibility of NPE about ReconfigurableBase.java

• Type: Improvement
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 3.0.0-alpha3
• Fix Version/s: 3.3.0
• Target Version/s: 3.3.0
• Assignee: hu xiaodong
• Reporter: hu xiaodong
• Created: 25/Aug/17 05:48
• Updated: 18/Jun/19 02:56
• Resolved: 18/Jun/19 02:56

Description

1.NameNode.java may throw a ReconfigurationException which getCause() is null

NameNode.java

  
  protected String reconfigurePropertyImpl(String property, String newVal)
      throws ReconfigurationException {
    final DatanodeManager datanodeManager = namesystem.getBlockManager()
        .getDatanodeManager();

    if (property.equals(DFS_HEARTBEAT_INTERVAL_KEY)) {
      return reconfHeartbeatInterval(datanodeManager, property, newVal);
    } else if (property.equals(DFS_NAMENODE_HEARTBEAT_RECHECK_INTERVAL_KEY)) {
      return reconfHeartbeatRecheckInterval(datanodeManager, property, newVal);
    } else if (property.equals(FS_PROTECTED_DIRECTORIES)) {
      return reconfProtectedDirectories(newVal);
    } else if (property.equals(HADOOP_CALLER_CONTEXT_ENABLED_KEY)) {
      return reconfCallerContextEnabled(newVal);
    } else if (property.equals(ipcClientRPCBackoffEnable)) {
      return reconfigureIPCBackoffEnabled(newVal);
    } 
   //=======================================================
   //here may throw a ReconfigurationException which getCause() is null
   //=======================================================
   else {
      throw new ReconfigurationException(property, newVal, getConf().get(
          property));
    }
  }

2. ReconfigurationThread.java will call ReconfigurationException.getCause().getMessage() which will cause NPE.

ReconfigurationThread.java

  
private static class ReconfigurationThread extends Thread {
    private ReconfigurableBase parent;

    ReconfigurationThread(ReconfigurableBase base) {
      this.parent = base;
    }

    // See {@link ReconfigurationServlet#applyChanges}
    public void run() {
      LOG.info("Starting reconfiguration task.");
      final Configuration oldConf = parent.getConf();
      final Configuration newConf = parent.getNewConf();
      final Collection<PropertyChange> changes =
          parent.getChangedProperties(newConf, oldConf);
      Map<PropertyChange, Optional<String>> results = Maps.newHashMap();
      ConfigRedactor oldRedactor = new ConfigRedactor(oldConf);
      ConfigRedactor newRedactor = new ConfigRedactor(newConf);
      for (PropertyChange change : changes) {
        String errorMessage = null;
        String oldValRedacted = oldRedactor.redact(change.prop, change.oldVal);
        String newValRedacted = newRedactor.redact(change.prop, change.newVal);
        if (!parent.isPropertyReconfigurable(change.prop)) {
          LOG.info(String.format(
              "Property %s is not configurable: old value: %s, new value: %s",
              change.prop,
              oldValRedacted,
              newValRedacted));
          continue;
        }
        LOG.info("Change property: " + change.prop + " from \""
            + ((change.oldVal == null) ? "<default>" : oldValRedacted)
            + "\" to \""
            + ((change.newVal == null) ? "<default>" : newValRedacted)
            + "\".");
        try {
          String effectiveValue =
              parent.reconfigurePropertyImpl(change.prop, change.newVal);
          if (change.newVal != null) {
            oldConf.set(change.prop, effectiveValue);
          } else {
            oldConf.unset(change.prop);
          }
        } catch (ReconfigurationException e) {
          //===============================================
          // here may occurs NPE,  because  e.getCause() may be null.
          //===============================================
          errorMessage = e.getCause().getMessage();
        }
        results.put(change, Optional.fromNullable(errorMessage));
      }

      synchronized (parent.reconfigLock) {
        parent.endTime = Time.now();
        parent.status = Collections.unmodifiableMap(results);
        parent.reconfigThread = null;
      }
    }
  }