You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/11/30 05:21:59 UTC
[GitHub] [hadoop] tasanuma commented on a change in pull request #3676: HDFS-16331. Make dfs.blockreport.intervalMsec reconfigurable
tasanuma commented on a change in pull request #3676:
URL: https://github.com/apache/hadoop/pull/3676#discussion_r758939114
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
##########
@@ -533,78 +536,119 @@ protected Configuration getNewConf() {
public String reconfigurePropertyImpl(String property, String newVal)
throws ReconfigurationException {
switch (property) {
- case DFS_DATANODE_DATA_DIR_KEY: {
- IOException rootException = null;
+ case DFS_DATANODE_DATA_DIR_KEY: {
+ IOException rootException = null;
+ try {
+ LOG.info("Reconfiguring {} to {}", property, newVal);
+ this.refreshVolumes(newVal);
+ return getConf().get(DFS_DATANODE_DATA_DIR_KEY);
+ } catch (IOException e) {
+ rootException = e;
+ } finally {
+ // Send a full block report to let NN acknowledge the volume changes.
try {
- LOG.info("Reconfiguring {} to {}", property, newVal);
- this.refreshVolumes(newVal);
- return getConf().get(DFS_DATANODE_DATA_DIR_KEY);
+ triggerBlockReport(
+ new BlockReportOptions.Factory().setIncremental(false).build());
} catch (IOException e) {
- rootException = e;
+ LOG.warn("Exception while sending the block report after refreshing"
+ + " volumes {} to {}", property, newVal, e);
+ if (rootException == null) {
+ rootException = e;
+ }
} finally {
- // Send a full block report to let NN acknowledge the volume changes.
- try {
- triggerBlockReport(
- new BlockReportOptions.Factory().setIncremental(false).build());
- } catch (IOException e) {
- LOG.warn("Exception while sending the block report after refreshing"
- + " volumes {} to {}", property, newVal, e);
- if (rootException == null) {
- rootException = e;
- }
- } finally {
- if (rootException != null) {
- throw new ReconfigurationException(property, newVal,
- getConf().get(property), rootException);
- }
+ if (rootException != null) {
+ throw new ReconfigurationException(property, newVal,
+ getConf().get(property), rootException);
}
}
- break;
}
- case DFS_DATANODE_BALANCE_MAX_NUM_CONCURRENT_MOVES_KEY: {
- ReconfigurationException rootException = null;
- try {
- LOG.info("Reconfiguring {} to {}", property, newVal);
- int movers;
- if (newVal == null) {
- // set to default
- movers = DFS_DATANODE_BALANCE_MAX_NUM_CONCURRENT_MOVES_DEFAULT;
- } else {
- movers = Integer.parseInt(newVal);
- if (movers <= 0) {
- rootException = new ReconfigurationException(
- property,
- newVal,
- getConf().get(property),
- new IllegalArgumentException(
- "balancer max concurrent movers must be larger than 0"));
- }
- }
- boolean success = xserver.updateBalancerMaxConcurrentMovers(movers);
- if (!success) {
+ break;
+ }
+ case DFS_DATANODE_BALANCE_MAX_NUM_CONCURRENT_MOVES_KEY: {
+ ReconfigurationException rootException = null;
+ try {
+ LOG.info("Reconfiguring {} to {}", property, newVal);
+ int movers;
+ if (newVal == null) {
+ // set to default
+ movers = DFS_DATANODE_BALANCE_MAX_NUM_CONCURRENT_MOVES_DEFAULT;
+ } else {
+ movers = Integer.parseInt(newVal);
+ if (movers <= 0) {
rootException = new ReconfigurationException(
property,
newVal,
getConf().get(property),
new IllegalArgumentException(
- "Could not modify concurrent moves thread count"));
+ "balancer max concurrent movers must be larger than 0"));
}
- return Integer.toString(movers);
- } catch (NumberFormatException nfe) {
+ }
+ boolean success = xserver.updateBalancerMaxConcurrentMovers(movers);
+ if (!success) {
rootException = new ReconfigurationException(
- property, newVal, getConf().get(property), nfe);
- } finally {
- if (rootException != null) {
- LOG.warn(String.format(
- "Exception in updating balancer max concurrent movers %s to %s",
- property, newVal), rootException);
- throw rootException;
+ property,
+ newVal,
+ getConf().get(property),
+ new IllegalArgumentException(
+ "Could not modify concurrent moves thread count"));
+ }
+ return Integer.toString(movers);
+ } catch (NumberFormatException nfe) {
+ rootException = new ReconfigurationException(
+ property, newVal, getConf().get(property), nfe);
+ } finally {
+ if (rootException != null) {
+ LOG.warn(String.format(
+ "Exception in updating balancer max concurrent movers %s to %s",
+ property, newVal), rootException);
+ throw rootException;
+ }
+ }
+ break;
+ }
+ case DFS_BLOCKREPORT_INTERVAL_MSEC_KEY: {
+ ReconfigurationException rootException = null;
+ try {
+ LOG.info("Reconfiguring {} to {}", property, newVal);
+ long intervalMs;
+ if (newVal == null) {
+ // Set to default.
+ intervalMs = DFS_BLOCKREPORT_INTERVAL_MSEC_DEFAULT;
+ } else {
+ intervalMs = Long.parseLong(newVal);
+ if (intervalMs < 0) {
+ rootException = new ReconfigurationException(
+ property,
+ newVal,
+ getConf().get(property),
+ new IllegalArgumentException(
+ "block report interval must be larger than or equal to 0"));
+ }
+ }
+ dnConf.setBlockReportInterval(intervalMs);
+ for (BPOfferService bpos : blockPoolManager.getAllNamenodeThreads()) {
+ if (bpos != null) {
+ for (BPServiceActor actor : bpos.getBPServiceActors()) {
+ actor.getScheduler().setBlockReportIntervalMs(intervalMs);
+ }
Review comment:
`intervalMs` still can be negative here. If `intervalMs` is negative, it should throw an exception before setting the value, shouldn't it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org