adoroszlai commented on a change in pull request #2786:
URL: https://github.com/apache/ozone/pull/2786#discussion_r749114442
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -356,6 +377,21 @@ public void setExcludeNodes(String excludeNodes) {
this.excludeNodes = excludeNodes;
}
+ /**
+ * Sets the datanodes that will be excluded from balancing.
+ * @param excludeNodes a File of datanode hostnames or ip addresses
+ * @throws IOException if an I/O error occurs when opening the file
+ */
+ public void setExcludeNodes(File excludeNodes) throws IOException {
+ try (Stream<String> strings = Files.lines(excludeNodes.toPath(),
+ StandardCharsets.UTF_8)) {
+ this.excludeNodes = strings.collect(Collectors.joining(","));
+ } catch (IOException e) {
+ LOG.debug("Could not read the specified excludeNodes file.");
+ throw new IOException(e);
Review comment:
Same comments apply here. BTW, can you please extract the logic
duplicated in these methods?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -332,6 +337,22 @@ public void setIncludeNodes(String includeNodes) {
this.includeNodes = includeNodes;
}
+ /**
+ * Sets the datanodes that will be the exclusive participants in balancing.
+ * Applicable only if the specified file is non-empty.
+ * @param includeNodes a File of datanode hostnames or ip addresses
+ * @throws IOException if an I/O error occurs when opening the file
+ */
+ public void setIncludeNodes(File includeNodes) throws IOException {
+ try (Stream<String> strings = Files.lines(includeNodes.toPath(),
+ StandardCharsets.UTF_8)) {
+ this.includeNodes = strings.collect(Collectors.joining(","));
+ } catch (IOException e) {
+ LOG.debug("Could not read the specified includeNodes file.");
+ throw new IOException(e);
Review comment:
No need to wrap it in another `IOException`.
```suggestion
throw e;
```
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -332,6 +337,22 @@ public void setIncludeNodes(String includeNodes) {
this.includeNodes = includeNodes;
}
+ /**
+ * Sets the datanodes that will be the exclusive participants in balancing.
+ * Applicable only if the specified file is non-empty.
+ * @param includeNodes a File of datanode hostnames or ip addresses
+ * @throws IOException if an I/O error occurs when opening the file
+ */
+ public void setIncludeNodes(File includeNodes) throws IOException {
+ try (Stream<String> strings = Files.lines(includeNodes.toPath(),
+ StandardCharsets.UTF_8)) {
+ this.includeNodes = strings.collect(Collectors.joining(","));
+ } catch (IOException e) {
+ LOG.debug("Could not read the specified includeNodes file.");
Review comment:
This message does not seem to be too helpful. First I need to enable
debug log level. Second, it does not include the filename.
```suggestion
LOG.info("Could not read includeNodes file: {}.",
includeNodes.getPath());
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
