adoroszlai commented on code in PR #8813:
URL: https://github.com/apache/ozone/pull/8813#discussion_r2221355339


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerReplicaPendingOps.java:
##########
@@ -59,12 +61,47 @@ public class ContainerReplicaPendingOps {
   private ReplicationManagerMetrics replicationMetrics = null;
   private final List<ContainerReplicaPendingOpsSubscriber> subscribers =
       new ArrayList<>();
+  // tracks how much data is pending to be added to a target Datanode because 
of pending ADD ops
+  private final ConcurrentHashMap<DatanodeID, SizeAndTime> 
containerSizeScheduled = new ConcurrentHashMap<>();
+  private final long replicationManagerEventTimeout;
 
-  public ContainerReplicaPendingOps(Clock clock) {
+  public ContainerReplicaPendingOps(Clock clock, OzoneConfiguration conf) {
     this.clock = clock;
+    ReplicationManager.ReplicationManagerConfiguration rmConf =
+        
conf.getObject(ReplicationManager.ReplicationManagerConfiguration.class);
+    replicationManagerEventTimeout = rmConf.getEventTimeout();

Review Comment:
   `ReplicationManagerConfiguration.eventTimeout` is reconfigurable (without 
server restart), but `ContainerReplicaPendingOps` will not pick up the updated 
value.  We need to share the instance of `ReplicationManagerConfiguration` 
between `ReplicationManager` and `ContainerReplicaPendingOps`.



##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/AbstractReconContainerManagerTest.java:
##########
@@ -109,7 +109,7 @@ public void setUp(@TempDir File tempDir) throws Exception {
         scmhaManager,
         scmContext);
     ContainerReplicaPendingOps pendingOps = new ContainerReplicaPendingOps(
-        Clock.system(ZoneId.systemDefault()));
+        Clock.system(ZoneId.systemDefault()), conf);

Review Comment:
   These changes can be avoided by overloading the constructor: keeping the 
existing one for unrelated tests, and adding the new parameter for prod and 
related tests.
   
   In a follow-up refactoring, we can replace the test-only constructor with a 
factory method in `ContainerReplicaPendingOps` for unrelated tests, thus also 
eliminating the need to pass "some clock".



-- 
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]

Reply via email to