sumitagrawl commented on code in PR #9243:
URL: https://github.com/apache/ozone/pull/9243#discussion_r2489436766
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperHelper.java:
##########
@@ -418,23 +444,21 @@ public static void handleKeyReprocess(String key,
// Save on writes. No need to save same container-key prefix mapping
again.
containerKeyMap.put(containerKeyPrefix, 1);
- // Check if container already exists; if not, increment the count
- if (!reconContainerMetadataManager.doesContainerExists(containerId)
- && !containerKeyCountMap.containsKey(containerId)) {
- containerCountToIncrement++;
- }
-
- // Update the count of keys for the given containerID
- long keyCount = containerKeyCountMap.getOrDefault(containerId,
-
reconContainerMetadataManager.getKeyCountForContainer(containerId));
-
- containerKeyCountMap.put(containerId, keyCount + 1);
+ // if it exists, update the count of keys for the given containerID
+ // else, increment the count of containers and initialize keyCount
+ containerKeyCountMap.compute(containerId, (k, v) -> {
Review Comment:
this is required for directory also add with process data in atomic way
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java:
##########
@@ -253,14 +254,17 @@ protected void handleDeleteDirEvent(OmDirectoryInfo
directoryInfo,
}
}
- protected boolean flushAndCommitNSToDB(Map<Long, NSSummary> nsSummaryMap) {
- try {
- updateNSSummariesToDB(nsSummaryMap, Collections.emptyList());
- } catch (IOException e) {
- LOG.error("Unable to write Namespace Summary data in Recon DB.", e);
- return false;
- } finally {
- nsSummaryMap.clear();
+ protected synchronized boolean flushAndCommitNSToDB(Map<Long, NSSummary>
nsSummaryMap,
+ long
nsSummaryFlushToDBMaxThreshold) {
+ if (nsSummaryMap.size() >= nsSummaryFlushToDBMaxThreshold) {
Review Comment:
we should not have this check at this point, caller is already having this
check
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java:
##########
@@ -87,34 +87,35 @@ protected void handlePutKeyEvent(OmKeyInfo keyInfo,
Map<Long,
NSSummary> nsSummaryMap) throws IOException {
long parentObjectId = keyInfo.getParentObjectID();
// Try to get the NSSummary from our local map that maps NSSummaries to IDs
- NSSummary nsSummary = nsSummaryMap.get(parentObjectId);
- if (nsSummary == null) {
- // If we don't have it in this batch we try to get it from the DB
- nsSummary = reconNamespaceSummaryManager.getNSSummary(parentObjectId);
- }
- if (nsSummary == null) {
- // If we don't have it locally and in the DB we create a new instance
- // as this is a new ID
- nsSummary = new NSSummary();
- }
- int[] fileBucket = nsSummary.getFileSizeBucket();
-
- // Update immediate parent's totals (includes all descendant files)
- nsSummary.setNumOfFiles(nsSummary.getNumOfFiles() + 1);
- nsSummary.setSizeOfFiles(nsSummary.getSizeOfFiles() +
keyInfo.getDataSize());
- // Before arithmetic operations, check for sentinel value
- long currentReplSize = nsSummary.getReplicatedSizeOfFiles();
- if (currentReplSize < 0) {
- // Old data, initialize to 0 before first use
- currentReplSize = 0;
- nsSummary.setReplicatedSizeOfFiles(0);
- }
- nsSummary.setReplicatedSizeOfFiles(currentReplSize +
keyInfo.getReplicatedSize());
- int binIndex = ReconUtils.getFileSizeBinIndex(keyInfo.getDataSize());
+ nsSummaryMap.compute(parentObjectId, (k, v) -> {
Review Comment:
NSSummary Object has multiple member fields for directory like file size,
file count, file bucket, childDir All needs to be threadsafe, else these value
can go wrong or exception for Set for concurrent modification.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OmTableInsightTask.java:
##########
@@ -108,6 +108,9 @@ public void init() {
*/
@Override
public TaskResult reprocess(OMMetadataManager omMetadataManager) {
+ LOG.info("Starting RocksDB Reprocess for {}", getTaskName());
Review Comment:
I think metrics are added for all task, do we need same in info log also
here at this granularity?
--
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]
