ArafatKhan2198 commented on code in PR #6272:
URL: https://github.com/apache/ozone/pull/6272#discussion_r1577405270


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainersInfoHandler.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+import jakarta.annotation.Nullable;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.recon.scm.ReconScmMetadataManager;
+import org.apache.hadoop.ozone.recon.spi.ReconContainerMetadataManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Manages Containers Table info of SCM MetaData DB, updates Recon's in-memory
+ * Container manager objects and global stats table to update container stats.
+ */
+public class ContainersInfoHandler implements SCMMetaDataTableHandler {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ContainersInfoHandler.class);
+
+  private ReconContainerMetadataManager reconContainerMetadataManager;
+  private ReconScmMetadataManager scmMetadataManager;
+  private Map<HddsProtos.LifeCycleState, Long> containerCountMap;
+
+  public ContainersInfoHandler(ReconContainerMetadataManager 
reconContainerMetadataManager,
+                               ReconScmMetadataManager scmMetadataManager,
+                               Map<HddsProtos.LifeCycleState, Long> 
containerCountMap) {
+    this.reconContainerMetadataManager = reconContainerMetadataManager;
+    this.scmMetadataManager = scmMetadataManager;
+    this.containerCountMap = containerCountMap;
+  }
+
+  private void handleUpdateContainerEvent(ContainerInfo containerInfo) throws 
IOException {
+    scmMetadataManager.getOzoneStorageContainerManager().getContainerManager()
+        .deleteContainer(ContainerID.valueOf(containerInfo.getContainerID()));
+    
scmMetadataManager.getOzoneStorageContainerManager().getContainerManager().initialize(containerInfo);
+  }
+
+  private void handleDeleteContainerEvent(ContainerInfo containerInfo)
+      throws IOException {
+    containerCountMap.compute(containerInfo.getState(), (k, v) -> (v != null ? 
v : 0L) - 1);
+    scmMetadataManager.getOzoneStorageContainerManager().getContainerManager()
+        .deleteContainer(ContainerID.valueOf(containerInfo.getContainerID()));
+  }
+
+  private void handlePutContainerEvent(ContainerInfo containerInfo) throws 
IOException {
+    containerCountMap.compute(containerInfo.getState(), (k, v) -> (v != null ? 
v : 0L) + 1);
+    
scmMetadataManager.getOzoneStorageContainerManager().getContainerManager().initialize(containerInfo);
+
+  }
+
+  /**
+   * Handles a PUT event on scm metadata DB tables.
+   *
+   * @param event The PUT event to be processed.
+   */
+  @Override
+  public void handlePutEvent(RocksDBUpdateEvent<?, Object> event) {
+    ContainerID containerId = (ContainerID) event.getKey();
+    ContainerInfo containerInfo = (ContainerInfo) event.getValue();
+    try {
+      handlePutContainerEvent(containerInfo);
+    } catch (IOException ioe) {
+      LOG.error("Unexpected error while handling add new container event and 
processing container stats for" +
+          " containerId: {} - ", containerId, ioe);
+    }
+  }
+
+  /**
+   * Handles a DELETE event on scm metadata DB tables.
+   *
+   * @param event The DELETE event to be processed.
+   */
+  @Override
+  public void handleDeleteEvent(RocksDBUpdateEvent<?, Object> event) {
+    ContainerID containerId = (ContainerID) event.getKey();
+    ContainerInfo containerInfo = (ContainerInfo) event.getValue();
+    try {
+      handleDeleteContainerEvent(containerInfo);
+    } catch (IOException ioe) {
+      LOG.error("Unexpected error while handling delete container event and 
processing container stats for" +
+          " containerId: {} - ", containerId, ioe);
+    }
+  }
+
+  /**
+   * Handles an UPDATE event on scm metadata DB tables.
+   *
+   * @param event The UPDATE event to be processed.
+   */
+  @Override
+  public void handleUpdateEvent(RocksDBUpdateEvent<?, Object> event) {
+    ContainerID containerId = (ContainerID) event.getKey();
+    ContainerInfo containerInfo = (ContainerInfo) event.getValue();
+    try {
+      handleUpdateContainerEvent(containerInfo);
+    } catch (Exception ioe) {
+      LOG.error("Unexpected error while handling update of container event and 
processing container stats for" +
+          " containerId: {} - ", containerId, ioe);
+    }
+  }
+
+  /**
+   * Iterates all the rows of desired SCM metadata DB table to capture
+   * and process the information further by sending to any downstream class.
+   *
+   * @param reconScmMetadataManager
+   * @return Pair represents the success (true) or failure (false) for the 
task.
+   * @throws IOException
+   */
+  @Override
+  public Pair<String, Boolean> reprocess(ReconScmMetadataManager 
reconScmMetadataManager) throws IOException {
+    long containerCount = 0;
+    Map<HddsProtos.LifeCycleState, Long> containerStateCountMap = new 
HashMap<>();
+    try {
+      LOG.info("Starting a 'reprocess' run of {}", getHandlerName());
+      Instant start = Instant.now();
+
+      // reset total count of deleted containers to zero
+      reconContainerMetadataManager.clearContainerStats();
+      Table<ContainerID, ContainerInfo> containerTable = 
reconScmMetadataManager.getContainerTable();
+      try (
+          TableIterator<ContainerID, ? extends Table.KeyValue<ContainerID, 
ContainerInfo>> containerTableIterator =
+              containerTable.iterator()) {
+        while (containerTableIterator.hasNext()) {
+          Table.KeyValue<ContainerID, ContainerInfo> keyValue = 
containerTableIterator.next();
+          ContainerInfo containerInfo = keyValue.getValue();
+          handleContainerStatsReprocess(containerInfo, containerStateCountMap);
+          containerCount++;
+        }
+        ImmutablePair<String, Boolean> result = 
saveContainerStats(containerStateCountMap);
+        if (result != null) {
+          return result;
+        }
+      }
+      LOG.info("Completed 'reprocess' of ContainerStatsTask.");
+      Instant end = Instant.now();
+      long duration = Duration.between(start, end).toMillis();
+      LOG.info("It took me {} seconds to process stats of {} containers.",
+          (double) duration / 1000.0, containerCount);
+    } catch (IOException ioEx) {
+      LOG.error("Unable to populate Container Stats data in Recon DB. ",
+          ioEx);
+      return new ImmutablePair<>(getHandlerName(), false);
+    }
+    return new ImmutablePair<>(getHandlerName(), true);
+  }
+
+  @Nullable
+  private ImmutablePair<String, Boolean> saveContainerStats(
+      Map<HddsProtos.LifeCycleState, Long> containerStateCountMap) {

Review Comment:
   Instead of using the fully qualified name `HddsProtos.LifeCycleState`, we 
can add it to the imports and just use LifeCycleState as the keys for the map 
making it more readable.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainersInfoHandler.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+import jakarta.annotation.Nullable;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.recon.scm.ReconScmMetadataManager;
+import org.apache.hadoop.ozone.recon.spi.ReconContainerMetadataManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Manages Containers Table info of SCM MetaData DB, updates Recon's in-memory
+ * Container manager objects and global stats table to update container stats.
+ */
+public class ContainersInfoHandler implements SCMMetaDataTableHandler {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ContainersInfoHandler.class);
+
+  private ReconContainerMetadataManager reconContainerMetadataManager;
+  private ReconScmMetadataManager scmMetadataManager;
+  private Map<HddsProtos.LifeCycleState, Long> containerCountMap;
+
+  public ContainersInfoHandler(ReconContainerMetadataManager 
reconContainerMetadataManager,
+                               ReconScmMetadataManager scmMetadataManager,
+                               Map<HddsProtos.LifeCycleState, Long> 
containerCountMap) {

Review Comment:
   Same here, and in other places where you have made changes.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainersInfoHandler.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+import jakarta.annotation.Nullable;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.recon.scm.ReconScmMetadataManager;
+import org.apache.hadoop.ozone.recon.spi.ReconContainerMetadataManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Manages Containers Table info of SCM MetaData DB, updates Recon's in-memory
+ * Container manager objects and global stats table to update container stats.
+ */
+public class ContainersInfoHandler implements SCMMetaDataTableHandler {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ContainersInfoHandler.class);
+
+  private ReconContainerMetadataManager reconContainerMetadataManager;
+  private ReconScmMetadataManager scmMetadataManager;
+  private Map<HddsProtos.LifeCycleState, Long> containerCountMap;

Review Comment:
   The variable name `containerCountMap` could be more descriptive. Consider 
renaming it to something like `containerStateCountMap` to better convey its 
purpose.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainersInfoHandler.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+import jakarta.annotation.Nullable;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.recon.scm.ReconScmMetadataManager;
+import org.apache.hadoop.ozone.recon.spi.ReconContainerMetadataManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Manages Containers Table info of SCM MetaData DB, updates Recon's in-memory
+ * Container manager objects and global stats table to update container stats.
+ */
+public class ContainersInfoHandler implements SCMMetaDataTableHandler {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ContainersInfoHandler.class);
+
+  private ReconContainerMetadataManager reconContainerMetadataManager;
+  private ReconScmMetadataManager scmMetadataManager;
+  private Map<HddsProtos.LifeCycleState, Long> containerCountMap;
+
+  public ContainersInfoHandler(ReconContainerMetadataManager 
reconContainerMetadataManager,
+                               ReconScmMetadataManager scmMetadataManager,
+                               Map<HddsProtos.LifeCycleState, Long> 
containerCountMap) {
+    this.reconContainerMetadataManager = reconContainerMetadataManager;
+    this.scmMetadataManager = scmMetadataManager;
+    this.containerCountMap = containerCountMap;
+  }
+
+  private void handleUpdateContainerEvent(ContainerInfo containerInfo) throws 
IOException {
+    scmMetadataManager.getOzoneStorageContainerManager().getContainerManager()
+        .deleteContainer(ContainerID.valueOf(containerInfo.getContainerID()));
+    
scmMetadataManager.getOzoneStorageContainerManager().getContainerManager().initialize(containerInfo);
+  }
+
+  private void handleDeleteContainerEvent(ContainerInfo containerInfo)
+      throws IOException {
+    containerCountMap.compute(containerInfo.getState(), (k, v) -> (v != null ? 
v : 0L) - 1);
+    scmMetadataManager.getOzoneStorageContainerManager().getContainerManager()
+        .deleteContainer(ContainerID.valueOf(containerInfo.getContainerID()));
+  }
+
+  private void handlePutContainerEvent(ContainerInfo containerInfo) throws 
IOException {
+    containerCountMap.compute(containerInfo.getState(), (k, v) -> (v != null ? 
v : 0L) + 1);
+    
scmMetadataManager.getOzoneStorageContainerManager().getContainerManager().initialize(containerInfo);
+
+  }
+
+  /**
+   * Handles a PUT event on scm metadata DB tables.
+   *
+   * @param event The PUT event to be processed.
+   */
+  @Override
+  public void handlePutEvent(RocksDBUpdateEvent<?, Object> event) {
+    ContainerID containerId = (ContainerID) event.getKey();
+    ContainerInfo containerInfo = (ContainerInfo) event.getValue();
+    try {
+      handlePutContainerEvent(containerInfo);
+    } catch (IOException ioe) {
+      LOG.error("Unexpected error while handling add new container event and 
processing container stats for" +
+          " containerId: {} - ", containerId, ioe);
+    }
+  }
+

Review Comment:
   Should we consider eliminating the separate method `handlePutContainerEvent` 
for `handlePutEvent`? Since the method is neither complex nor reused elsewhere, 
merging `handlePutEvent` and `handlePutContainerEvent` could simplify and 
enhance readability. We might apply the same approach to the DELETE and UPDATE 
events as well.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/PipelineInfoHandler.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
+import org.apache.hadoop.ozone.ClientVersion;
+import org.apache.hadoop.ozone.recon.scm.ReconScmMetadataManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+/**
+ * Manages Pipeline Table info of SCM MetaData DB, updates Recon's in-memory
+ * Pipeline manager objects.
+ */
+public class PipelineInfoHandler implements SCMMetaDataTableHandler {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(PipelineInfoHandler.class);
+
+  private ReconScmMetadataManager scmMetadataManager;
+
+  public PipelineInfoHandler(ReconScmMetadataManager scmMetadataManager) {
+    this.scmMetadataManager = scmMetadataManager;
+  }
+
+  /**
+   * Handles a PUT event on scm metadata DB tables.
+   *
+   * @param event The PUT event to be processed.
+   */
+  @Override
+  public void handlePutEvent(RocksDBUpdateEvent<?, Object> event) {
+    PipelineID pipelineID = (PipelineID) event.getKey();
+    Pipeline pipeline = (Pipeline) event.getValue();
+    try {
+      LOG.info("handlePutEvent for add new pipeline event for pipelineId: {}", 
pipelineID);
+      handlePutPipelineEvent(pipeline);
+    } catch (IOException ioe) {
+      LOG.error("Unexpected error while handling add new pipeline event for 
pipelineId: {} - ", pipelineID, ioe);
+    }
+  }
+
+  private void handlePutPipelineEvent(Pipeline pipeline) throws IOException {
+    
scmMetadataManager.getOzoneStorageContainerManager().getPipelineManager().initialize(pipeline);
+  }

Review Comment:
   Should we consider eliminating the separate method `handlePutPipelineEvent` 
for `handlePutEvent`? Since the method is neither complex nor reused elsewhere, 
merging handlePutEvent and handlePutPipelineEvent could simplify and enhance 
readability. We might apply the same approach to the `DELETE` and `UPDATE` 
events as well.



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