adoroszlai commented on code in PR #9464:
URL: https://github.com/apache/ozone/pull/9464#discussion_r2620978402
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java:
##########
@@ -340,6 +341,23 @@ void reportDatanode(DatanodeDetails dn) throws IOException
{
nodeStatus.put(dn, System.currentTimeMillis());
}
+ public List<DatanodeDetails> removeAllFromNodeStatus() {
+ Iterator<DatanodeDetails> iterator = nodeStatus.keySet().iterator();
+ List<DatanodeDetails> keys = new ArrayList<>();
+ while (iterator.hasNext()) {
+ DatanodeDetails firstKey = iterator.next();
+ keys.add(firstKey);
+ }
+ for (DatanodeDetails dn : keys) {
+ nodeStatus.remove(dn);
+ }
+ return keys;
+ }
+
+ public void setInNodeStatus(List<DatanodeDetails> dd) {
+ nodeStatus.put(dd.get(0), Time.now());
+ }
Review Comment:
These methods to manipulate state for the test look dangerous (for prod.)
and fragile (as they cater for a single test case).
I think it would be more safe to:
- make the class non-`final`
- create test-specific subclass
- use `PipelineProvider` to create instances of the test-specific subclass
In any case, the logic in `removeAllFromNodeStatus` can be simplified:
```java
List<DatanodeDetails> keys = getNodes();
nodeStatus.clear(); // or nodeStatus.keySet().removeAll(keys);
return keys;
```
--
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]
