sumitagrawl commented on code in PR #4626: URL: https://github.com/apache/ozone/pull/4626#discussion_r1219797915
########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/CleanUpTask.java: ########## @@ -0,0 +1,190 @@ +/* + * 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 com.google.inject.Inject; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.hadoop.hdds.utils.db.DBStore; +import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.WithObjectID; +import org.apache.hadoop.ozone.recon.api.types.NSSummary; +import org.apache.hadoop.ozone.recon.api.types.OrphanKeyMetaData; +import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager; +import org.apache.hadoop.ozone.recon.spi.impl.ReconDBProvider; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; + +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.BUCKET_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DELETED_DIR_TABLE; +import static org.apache.hadoop.ozone.recon.spi.impl.ReconDBDefinition.ORPHAN_KEYS_METADATA; + +/** + * Task class to iterate over the OM DB and cleanup below data:. + * 1. namespaceSummaryTable data. + * 2. orphan_keys_metadata table data. + */ +public class CleanUpTask implements ReconOmTask { + + private static final Logger LOG = + LoggerFactory.getLogger(CleanUpTask.class); + private final DBStore reconDbStore; + private ReconNamespaceSummaryManager reconNamespaceSummaryManager; + private final Table<Long, OrphanKeyMetaData> orphanKeysMetaDataTable; + + @Inject + public CleanUpTask( + ReconDBProvider reconDBProvider, + ReconNamespaceSummaryManager reconNamespaceSummaryManager) + throws IOException { + this.reconDbStore = reconDBProvider.getDbStore(); + this.reconNamespaceSummaryManager = reconNamespaceSummaryManager; + this.orphanKeysMetaDataTable = + ORPHAN_KEYS_METADATA.getTable(reconDbStore); + } + + @Override + public String getTaskName() { + return "OrphanKeyDetectionTask"; + } + + public Collection<String> getTaskTables() { + List<String> taskTables = new ArrayList<>(); + taskTables.add(DELETED_DIR_TABLE); + taskTables.add(BUCKET_TABLE); + return taskTables; + } + + /** + * Process a set of OM events on tables that the task is listening on. + * + * @param events Set of events to be processed by the task. + * @return Pair of task name -> task success. + */ + @Override + public Pair<String, Boolean> process(OMUpdateEventBatch events) { + Iterator<OMDBUpdateEvent> eventIterator = events.getIterator(); + final Collection<String> taskTables = getTaskTables(); + while (eventIterator.hasNext()) { + OMDBUpdateEvent<String, ? extends + WithObjectID> omdbUpdateEvent = eventIterator.next(); + OMDBUpdateEvent.OMDBUpdateAction action = omdbUpdateEvent.getAction(); + // we only process updates on OM's deletedDirectoryTable + String table = omdbUpdateEvent.getTable(); + if (!taskTables.contains(table)) { + continue; + } + boolean updateOnBucketTable = table.equals(BUCKET_TABLE); + + try { + if (table.equals(DELETED_DIR_TABLE)) { + // key update on deletedDirectoryTable + OMDBUpdateEvent<String, OmKeyInfo> deletedDirTableUpdateEvent = + (OMDBUpdateEvent<String, OmKeyInfo>) omdbUpdateEvent; + OmKeyInfo updatedKeyInfo = deletedDirTableUpdateEvent.getValue(); + + switch (action) { + case PUT: + handlePutDeleteDirEvent(updatedKeyInfo); + break; + case DELETE: + case UPDATE: + break; + default: + LOG.debug("Skipping DB update event : {}", + omdbUpdateEvent.getAction()); + } + } + if (table.equals(BUCKET_TABLE)) { + // key update on Bucket Table + OMDBUpdateEvent<String, OmBucketInfo> bucketTableUpdateEvent = + (OMDBUpdateEvent<String, OmBucketInfo>) omdbUpdateEvent; + OmBucketInfo updatedBucketInfo = bucketTableUpdateEvent.getValue(); + + switch (action) { + case PUT: + case UPDATE: + break; + case DELETE: + handleBucketDeleteEvent(updatedBucketInfo); + break; + default: + LOG.debug("Skipping DB update event : {}", + omdbUpdateEvent.getAction()); + } + } + } catch (Exception ex) { + LOG.error("Unable to process Namespace Summary data in Recon DB. ", ex); + return new ImmutablePair<>(getTaskName(), false); + } + } + return new ImmutablePair<>(getTaskName(), true); + } + + private void handleBucketDeleteEvent(OmBucketInfo updatedBucketInfo) { + long objectID = updatedBucketInfo.getObjectID(); + removeOrphanAndNSSummaryParentEntry(objectID); + } + + private void removeOrphanAndNSSummaryParentEntry(long objectID) { + try { + OrphanKeyMetaData orphanKeyMetaData = + orphanKeysMetaDataTable.get(objectID); + if (null != orphanKeyMetaData) { + orphanKeysMetaDataTable.delete(objectID); + } + NSSummary nsSummary = reconNamespaceSummaryManager.getNSSummary(objectID); + if (null != nsSummary) { + reconNamespaceSummaryManager.deleteNSSummary(objectID); Review Comment: - If bucket is removed, removal from NSSummary is ok. But we may need this to orphan if fileCount is not "0" and sub-dir list is not "0". - If directory is added to deletedTable, do we need parentId from NSSummary? I think in re-process, we are adding it as dummy parentId. So we should not remove in this case. But removal from orphan is ok. - May need handle events, if directory is deleted from deletedTable, then we may need add to orphan if fileCount is not "0" and sub-dir is not "0". Also need remove as child getting its parent. - Need remove from nssummary only if all sub-dir list empty and fileCount "0". - We may need handle reducing fileCount by handling delete File event. ########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java: ########## @@ -225,4 +336,136 @@ protected boolean checkAndCallFlushToDB( } return true; } + + protected boolean writeFlushAndCommitOrphanKeysMetaDataToDB( + Map<Long, OrphanKeyMetaData> orphanKeyMetaDataMap, long status) { + try { + writeOrphanKeysMetaDataToDB(orphanKeyMetaDataMap, status); + orphanKeyMetaDataMap.clear(); + } catch (IOException e) { + LOG.error("Unable to write orphan keys meta data in Recon DB.", e); + return false; + } + return true; + } + + protected boolean checkOrphanDataAndCallWriteFlushToDB( + Map<Long, OrphanKeyMetaData> orphanKeyMetaDataMap, long status) { + // if map contains more than entries, flush to DB and clear the map + if (null != orphanKeyMetaDataMap && orphanKeyMetaDataMap.size() >= + orphanKeysFlushToDBMaxThreshold) { + return writeFlushAndCommitOrphanKeysMetaDataToDB( + orphanKeyMetaDataMap, status); + } + return true; + } + + protected void deleteOrphanKeysMetaDataFromDB( + List<Long> orphanKeysParentIdList) throws IOException { + try (RDBBatchOperation rdbBatchOperation = new RDBBatchOperation()) { + orphanKeysParentIdList.forEach(parentId -> { + try { + reconNamespaceSummaryManager.batchDeleteOrphanKeyMetaData( + rdbBatchOperation, parentId); + } catch (IOException e) { + LOG.error( Review Comment: This method never throw exception, so caller will return always true. ########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/CleanUpTask.java: ########## @@ -0,0 +1,190 @@ +/* + * 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 com.google.inject.Inject; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.hadoop.hdds.utils.db.DBStore; +import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.WithObjectID; +import org.apache.hadoop.ozone.recon.api.types.NSSummary; +import org.apache.hadoop.ozone.recon.api.types.OrphanKeyMetaData; +import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager; +import org.apache.hadoop.ozone.recon.spi.impl.ReconDBProvider; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; + +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.BUCKET_TABLE; +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DELETED_DIR_TABLE; +import static org.apache.hadoop.ozone.recon.spi.impl.ReconDBDefinition.ORPHAN_KEYS_METADATA; + +/** + * Task class to iterate over the OM DB and cleanup below data:. + * 1. namespaceSummaryTable data. + * 2. orphan_keys_metadata table data. + */ +public class CleanUpTask implements ReconOmTask { + + private static final Logger LOG = + LoggerFactory.getLogger(CleanUpTask.class); + private final DBStore reconDbStore; + private ReconNamespaceSummaryManager reconNamespaceSummaryManager; + private final Table<Long, OrphanKeyMetaData> orphanKeysMetaDataTable; + + @Inject + public CleanUpTask( + ReconDBProvider reconDBProvider, + ReconNamespaceSummaryManager reconNamespaceSummaryManager) + throws IOException { + this.reconDbStore = reconDBProvider.getDbStore(); + this.reconNamespaceSummaryManager = reconNamespaceSummaryManager; + this.orphanKeysMetaDataTable = + ORPHAN_KEYS_METADATA.getTable(reconDbStore); + } + + @Override + public String getTaskName() { + return "OrphanKeyDetectionTask"; + } + + public Collection<String> getTaskTables() { + List<String> taskTables = new ArrayList<>(); + taskTables.add(DELETED_DIR_TABLE); + taskTables.add(BUCKET_TABLE); + return taskTables; + } + + /** + * Process a set of OM events on tables that the task is listening on. + * + * @param events Set of events to be processed by the task. + * @return Pair of task name -> task success. + */ + @Override + public Pair<String, Boolean> process(OMUpdateEventBatch events) { + Iterator<OMDBUpdateEvent> eventIterator = events.getIterator(); + final Collection<String> taskTables = getTaskTables(); + while (eventIterator.hasNext()) { + OMDBUpdateEvent<String, ? extends + WithObjectID> omdbUpdateEvent = eventIterator.next(); + OMDBUpdateEvent.OMDBUpdateAction action = omdbUpdateEvent.getAction(); + // we only process updates on OM's deletedDirectoryTable + String table = omdbUpdateEvent.getTable(); + if (!taskTables.contains(table)) { + continue; + } + boolean updateOnBucketTable = table.equals(BUCKET_TABLE); Review Comment: un-used variable ########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java: ########## @@ -225,4 +328,146 @@ protected boolean checkAndCallFlushToDB( } return true; } + + protected boolean writeFlushAndCommitOrphanKeysMetaDataToDB( + Map<Long, OrphanKeyMetaData> orphanKeyMetaDataMap, long status) { + try { + writeOrphanKeysMetaDataToDB(orphanKeyMetaDataMap, status); + orphanKeyMetaDataMap.clear(); + } catch (IOException e) { + LOG.error("Unable to write orphan keys meta data in Recon DB.", e); + return false; + } + return true; + } + + protected boolean checkOrphanDataAndCallWriteFlushToDB( + Map<Long, OrphanKeyMetaData> orphanKeyMetaDataMap, long status) { + // if map contains more than entries, flush to DB and clear the map + if (null != orphanKeyMetaDataMap && orphanKeyMetaDataMap.size() >= + orphanKeysFlushToDBMaxThreshold) { + return writeFlushAndCommitOrphanKeysMetaDataToDB( + orphanKeyMetaDataMap, status); + } + return true; + } + + protected void deleteOrphanKeysMetaDataFromDB( + List<Long> orphanKeysParentIdList) throws IOException { + try (RDBBatchOperation rdbBatchOperation = new RDBBatchOperation()) { + orphanKeysParentIdList.forEach(parentId -> { + try { + reconNamespaceSummaryManager.batchDeleteOrphanKeyMetaData( + rdbBatchOperation, parentId); + } catch (IOException e) { + LOG.error( + "Unable to delete orphan keys from orphanKeysMetaDataTable " + + "in Recon DB.", e); + } + }); + try { + reconNamespaceSummaryManager.commitBatchOperation(rdbBatchOperation); + } catch (IOException e) { + // Logging as Info as we don't want to log as error when any dir not + // found in orphan candidate metadata set. This is done to avoid 2 + // rocks DB operations - check if present and then delete operation. + LOG.info("Delete batch unable to delete few entries as dir may not be" + + " found in orphan candidate metadata set"); + } + } + } + + protected boolean batchDeleteAndCommitOrphanKeysMetaDataToDB( + List<Long> orphanKeysParentIdList) { + try { + deleteOrphanKeysMetaDataFromDB(orphanKeysParentIdList); + orphanKeysParentIdList.clear(); + } catch (IOException e) { + LOG.error("Unable to delete orphan keys meta data from Recon DB.", e); + return false; + } + return true; + } + + protected boolean checkOrphanDataThresholdAndAddToDeleteBatch( + List<Long> orphanKeysParentIdList) { + // if map contains more than entries, flush to DB and clear the map + if (null != orphanKeysParentIdList && orphanKeysParentIdList.size() >= + orphanKeysFlushToDBMaxThreshold) { + return batchDeleteAndCommitOrphanKeysMetaDataToDB(orphanKeysParentIdList); + } + return true; + } + + private <T extends WithParentObjectId> void addOrphanCandidate( + T fileDirObjInfo, + Map<Long, OrphanKeyMetaData> orphanKeyMetaDataMap, + long status, + boolean parentExist) + throws IOException { + if (null != orphanKeyMetaDataMap) { + long objectID = fileDirObjInfo.getObjectID(); + long parentObjectID = fileDirObjInfo.getParentObjectID(); + if (parentExist) { + OrphanKeyMetaData orphanKeyMetaData = + orphanKeyMetaDataMap.get(parentObjectID); + if (null == orphanKeyMetaData) { + orphanKeyMetaData = + reconNamespaceSummaryManager.getOrphanKeyMetaData( + parentObjectID); + } + if (null != orphanKeyMetaData) { + Set<Long> objectIds = orphanKeyMetaData.getObjectIds(); + objectIds.add(objectID); + orphanKeyMetaDataMap.put(parentObjectID, orphanKeyMetaData); + } + } else { + Set<Long> objectIds = new HashSet<>(); + objectIds.add(objectID); + OrphanKeyMetaData orphanKeyMetaData = + new OrphanKeyMetaData(objectIds, status); + orphanKeyMetaDataMap.put(parentObjectID, orphanKeyMetaData); + } + } + } + + protected boolean verifyOrphanParentsForBucket( + Set<Long> bucketObjectIdsSet, + List<Long> toBeDeletedBucketObjectIdsFromOrphanMap) + throws IOException { + try (TableIterator<Long, ? extends Table.KeyValue<Long, + OrphanKeyMetaData>> orphanKeysMetaDataIter = + orphanKeysMetaDataTable.iterator()) { + while (orphanKeysMetaDataIter.hasNext()) { + Table.KeyValue<Long, OrphanKeyMetaData> keyValue = + orphanKeysMetaDataIter.next(); + Long parentId = keyValue.getKey(); + if (bucketObjectIdsSet.contains(parentId)) { + toBeDeletedBucketObjectIdsFromOrphanMap.add(parentId); + if (!checkOrphanDataThresholdAndAddToDeleteBatch( + toBeDeletedBucketObjectIdsFromOrphanMap)) { + return true; Review Comment: Base method called in its sub-hierarchy deleteOrphanKeysMetaDataFromDB() does not throw exception, it just log. ########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskWithLegacy.java: ########## @@ -120,23 +124,27 @@ public boolean processWithLegacy(OMUpdateEventBatch events) { if (!updatedKeyInfo.getKeyName().endsWith(OM_KEY_PREFIX)) { switch (action) { case PUT: - handlePutKeyEvent(updatedKeyInfo, nsSummaryMap); + handlePutKeyEvent(updatedKeyInfo, nsSummaryMap, + new HashMap<>(), 1L); Review Comment: - There can be some db event if not passed as null, like delete may check db for exist, ... - earlier seems this can be null, but not with this does not seems this map will be null -- 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]
