devmadhuu commented on code in PR #4696:
URL: https://github.com/apache/ozone/pull/4696#discussion_r1192308811


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -383,4 +445,63 @@ private void handlePutOMKeyEvent(String key, OmKeyInfo 
omKeyInfo,
     }
   }
 
+  /**
+   * Write an OM key to container DB and update containerID -> no. of keys
+   * count to the Global Stats table.
+   *
+   * @param key key String
+   * @param omKeyInfo omKeyInfo value
+   * @param containerKeyMap we keep the added containerKeys in this map
+   *                        to allow incremental batching to containerKeyTable
+   * @param containerKeyCountMap we keep the containerKey counts in this map 
+   *                             to allow batching to containerKeyCountTable
+   *                             after reprocessing is done
+   * @throws IOException if unable to write to recon DB.
+   */
+  private void handleKeyReprocess(String key,
+                                  OmKeyInfo omKeyInfo,
+                                  Map<ContainerKeyPrefix, Integer>
+                                      containerKeyMap,
+                                  Map<Long, Long> containerKeyCountMap)
+      throws IOException {
+    long containerCountToIncrement = 0;
+    for (OmKeyLocationInfoGroup omKeyLocationInfoGroup : omKeyInfo
+        .getKeyLocationVersions()) {
+      long keyVersion = omKeyLocationInfoGroup.getVersion();
+      for (OmKeyLocationInfo omKeyLocationInfo : omKeyLocationInfoGroup
+          .getLocationList()) {
+        long containerId = omKeyLocationInfo.getContainerID();
+        ContainerKeyPrefix containerKeyPrefix = new ContainerKeyPrefix(
+            containerId, key, keyVersion);
+        if (reconContainerMetadataManager.getCountForContainerKeyPrefix(
+            containerKeyPrefix) == 0) {

Review Comment:
   we can avoid the put operation by adding back the condition 
"`!containerKeyMap.containsKey(containerKeyPrefix)`"



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -120,14 +145,42 @@ public Pair<String, Boolean> reprocess(OMMetadataManager 
omMetadataManager) {
       return new ImmutablePair<>(getTaskName(), false);
     }
     try {
-      writeToTheDB(containerKeyMap, containerKeyCountMap, deletedKeyCountList);
+      // only batch write containerKeyCountMap to the containerKeyCountTable
+      // containerKeyMap has been written to the DB
+      // deleted container list is not needed since "reprocess" only has
+      // put operations
+      writeToTheDB(Collections.emptyMap(), containerKeyCountMap,
+          Collections.emptyList());
     } catch (IOException e) {
       LOG.error("Unable to write Container Key Prefix data in Recon DB.", e);
       return new ImmutablePair<>(getTaskName(), false);
     }
     return new ImmutablePair<>(getTaskName(), true);
   }
 
+  private boolean flushAndCommitContainerKeyMapToDB(
+      Map<ContainerKeyPrefix, Integer> containerKeyMap) {
+    try {
+      writeToTheDB(containerKeyMap, Collections.emptyMap(),

Review Comment:
   I think, we should flush the "`containerKeyCountMap`" also and optimize more 
memory footprint if we are introducing this flush mechanism, so you can 
refactor the code a bit and pass "`containerKeyCountMap`" instead of sending 
empty map in 2nd argument.



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