Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ChenSammi merged PR #9635: URL: https://github.com/apache/ozone/pull/9635 -- 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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ChenSammi commented on PR #9635: URL: https://github.com/apache/ozone/pull/9635#issuecomment-3798081906 Thanks @priyeshkaratha , and @ashishkumar50 @sadanand48 for the review. -- 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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
priyeshkaratha commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2720129071
##
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java:
##
@@ -246,6 +248,60 @@ void checkIfDeleteServiceIsDeletingKeys()
}
}
+/**
+ * Test that verifies zero-sized keys (keys with no blocks) are not sent
to SCM.
+ * The KeyDeletingService should filter out empty keys before calling SCM.
+ */
+@Test
+void checkIfDeleteServiceIsDeletingZeroSizedKeys()
+throws IOException, TimeoutException, InterruptedException {
+ // Spy on the SCM client to verify it's not called for empty keys
+ ScmBlockLocationTestingClient scmClientSpy =
Mockito.spy(scmBlockTestingClient);
+ // Create a KeyDeletingService with the spied client
+ KeyDeletingService testService = new KeyDeletingService(
+ om, scmClientSpy, 100, 1, conf, 10, false);
+ // Create a BlockGroup with empty deleted blocks list (zero-sized key)
+ BlockGroup blockGroup = BlockGroup.newBuilder().setKeyName("key1/1")
+ .addAllDeletedBlocks(new ArrayList<>()).build();
+ Map blockGroups = Collections.singletonMap(
+ blockGroup.getGroupID(),
+ new PurgedKey("vol", "buck", 1, blockGroup, "key1", 0, true));
+ // Process the key deletion
+ testService.processKeyDeletes(blockGroups, new HashMap<>(), new
ArrayList<>(), null, null);
+ // Verify that SCM's deleteKeyBlocks was never called (empty keys are
filtered out)
+ verify(scmClientSpy, never()).deleteKeyBlocks(any());
+ // Cleanup
+ testService.shutdown();
+}
+
+@Test
+void checkIfDeleteServiceIsDeletingMixedSizedKeys()
+throws IOException, TimeoutException, InterruptedException {
+ // Spy on the SCM client to verify it's not called for empty keys
+ ScmBlockLocationTestingClient scmClientSpy =
Mockito.spy(scmBlockTestingClient);
+ // Create a KeyDeletingService with the spied client
+ KeyDeletingService testService = new KeyDeletingService(
+ om, scmClientSpy, 100, 1, conf, 10, false);
+ // Create a BlockGroup with empty deleted blocks list (zero-sized key)
+ BlockGroup blockGroup1 = BlockGroup.newBuilder().setKeyName("key1/1")
+ .addAllDeletedBlocks(new ArrayList<>()).build();
+ //Create a BlockGroup with non-empty deleted blocks
+ List deletedBlocks = Collections.singletonList(new
DeletedBlock(new BlockID(1, 1), 1, 3));
+ BlockGroup blockGroup2 = BlockGroup.newBuilder().setKeyName("key2/2")
+ .addAllDeletedBlocks(deletedBlocks).build();
+ Map blockGroups = new HashMap<>();
+
+ blockGroups.put(blockGroup1.getGroupID(), new PurgedKey("vol", "buck",
1, blockGroup1, "key1", 0, true));
+ blockGroups.put(blockGroup2.getGroupID(), new PurgedKey("vol", "buck",
1, blockGroup2, "key2", 0, true));
+
+ // Process the key deletion
+ testService.processKeyDeletes(blockGroups, new HashMap<>(), new
ArrayList<>(), null, null);
+ // Verify that SCM's deleteKeyBlocks was never called (empty keys are
filtered out)
Review Comment:
Addressed in latest commit
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ChenSammi commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2719984738
##
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java:
##
@@ -246,6 +248,60 @@ void checkIfDeleteServiceIsDeletingKeys()
}
}
+/**
+ * Test that verifies zero-sized keys (keys with no blocks) are not sent
to SCM.
+ * The KeyDeletingService should filter out empty keys before calling SCM.
+ */
+@Test
+void checkIfDeleteServiceIsDeletingZeroSizedKeys()
+throws IOException, TimeoutException, InterruptedException {
+ // Spy on the SCM client to verify it's not called for empty keys
+ ScmBlockLocationTestingClient scmClientSpy =
Mockito.spy(scmBlockTestingClient);
+ // Create a KeyDeletingService with the spied client
+ KeyDeletingService testService = new KeyDeletingService(
+ om, scmClientSpy, 100, 1, conf, 10, false);
+ // Create a BlockGroup with empty deleted blocks list (zero-sized key)
+ BlockGroup blockGroup = BlockGroup.newBuilder().setKeyName("key1/1")
+ .addAllDeletedBlocks(new ArrayList<>()).build();
+ Map blockGroups = Collections.singletonMap(
+ blockGroup.getGroupID(),
+ new PurgedKey("vol", "buck", 1, blockGroup, "key1", 0, true));
+ // Process the key deletion
+ testService.processKeyDeletes(blockGroups, new HashMap<>(), new
ArrayList<>(), null, null);
+ // Verify that SCM's deleteKeyBlocks was never called (empty keys are
filtered out)
+ verify(scmClientSpy, never()).deleteKeyBlocks(any());
+ // Cleanup
+ testService.shutdown();
+}
+
+@Test
+void checkIfDeleteServiceIsDeletingMixedSizedKeys()
+throws IOException, TimeoutException, InterruptedException {
+ // Spy on the SCM client to verify it's not called for empty keys
+ ScmBlockLocationTestingClient scmClientSpy =
Mockito.spy(scmBlockTestingClient);
+ // Create a KeyDeletingService with the spied client
+ KeyDeletingService testService = new KeyDeletingService(
+ om, scmClientSpy, 100, 1, conf, 10, false);
+ // Create a BlockGroup with empty deleted blocks list (zero-sized key)
+ BlockGroup blockGroup1 = BlockGroup.newBuilder().setKeyName("key1/1")
+ .addAllDeletedBlocks(new ArrayList<>()).build();
+ //Create a BlockGroup with non-empty deleted blocks
+ List deletedBlocks = Collections.singletonList(new
DeletedBlock(new BlockID(1, 1), 1, 3));
+ BlockGroup blockGroup2 = BlockGroup.newBuilder().setKeyName("key2/2")
+ .addAllDeletedBlocks(deletedBlocks).build();
+ Map blockGroups = new HashMap<>();
+
+ blockGroups.put(blockGroup1.getGroupID(), new PurgedKey("vol", "buck",
1, blockGroup1, "key1", 0, true));
+ blockGroups.put(blockGroup2.getGroupID(), new PurgedKey("vol", "buck",
1, blockGroup2, "key2", 0, true));
+
+ // Process the key deletion
+ testService.processKeyDeletes(blockGroups, new HashMap<>(), new
ArrayList<>(), null, null);
+ // Verify that SCM's deleteKeyBlocks was never called (empty keys are
filtered out)
Review Comment:
Please remove this copy past comment.
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
priyeshkaratha commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2711087754
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,21 +139,49 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty keys): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
LOG.info("Send {} key(s) to SCM, first {} keys: {}",
keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
.map(Map.Entry::getValue).collect(Collectors.toSet()));
}
-List blockDeletionResults =
-scmClient.deleteKeyBlocks(keyBlocksList.values().stream()
-.map(PurgedKey::getBlockGroup).collect(Collectors.toList()));
+List blockDeletionResults;
+if (nonEmptyKeyBlocksList.isEmpty()) {
+ // Skip SCM call if all files are empty
+ blockDeletionResults = new ArrayList<>();
+ LOG.info("Skipping SCM call as all {} keys are empty",
keyBlocksList.size());
+} else {
+ blockDeletionResults =
+ scmClient.deleteKeyBlocks(nonEmptyKeyBlocksList.values().stream()
+ .map(PurgedKey::getBlockGroup).collect(Collectors.toList()));
+}
+
+// Add successful results for empty files (no need to send to SCM)
+Map emptyKeyBlocksList =
keyBlocksList.entrySet().stream()
Review Comment:
Addressed the changes
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
priyeshkaratha commented on code in PR #9635: URL: https://github.com/apache/ozone/pull/9635#discussion_r2711087238 ## hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java: ## @@ -246,6 +247,32 @@ void checkIfDeleteServiceIsDeletingKeys() } } +/** + * Test that verifies zero-sized keys (keys with no blocks) are not sent to SCM. + * The KeyDeletingService should filter out empty keys before calling SCM. + */ +@Test +void checkIfDeleteServiceIsDeletingZeroSizedKeys() Review Comment: Addressed -- 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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ChenSammi commented on code in PR #9635: URL: https://github.com/apache/ozone/pull/9635#discussion_r2707091319 ## hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java: ## @@ -246,6 +247,32 @@ void checkIfDeleteServiceIsDeletingKeys() } } +/** + * Test that verifies zero-sized keys (keys with no blocks) are not sent to SCM. + * The KeyDeletingService should filter out empty keys before calling SCM. + */ +@Test +void checkIfDeleteServiceIsDeletingZeroSizedKeys() Review Comment: Please add one more case for mixed file size case. ## hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java: ## @@ -246,6 +247,32 @@ void checkIfDeleteServiceIsDeletingKeys() } } +/** + * Test that verifies zero-sized keys (keys with no blocks) are not sent to SCM. + * The KeyDeletingService should filter out empty keys before calling SCM. + */ +@Test +void checkIfDeleteServiceIsDeletingZeroSizedKeys() Review Comment: Please add one more test for mixed file size case. -- 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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ChenSammi commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2707086810
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,21 +139,49 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty keys): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
LOG.info("Send {} key(s) to SCM, first {} keys: {}",
keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
.map(Map.Entry::getValue).collect(Collectors.toSet()));
}
-List blockDeletionResults =
-scmClient.deleteKeyBlocks(keyBlocksList.values().stream()
-.map(PurgedKey::getBlockGroup).collect(Collectors.toList()));
+List blockDeletionResults;
+if (nonEmptyKeyBlocksList.isEmpty()) {
+ // Skip SCM call if all files are empty
+ blockDeletionResults = new ArrayList<>();
+ LOG.info("Skipping SCM call as all {} keys are empty",
keyBlocksList.size());
+} else {
+ blockDeletionResults =
+ scmClient.deleteKeyBlocks(nonEmptyKeyBlocksList.values().stream()
+ .map(PurgedKey::getBlockGroup).collect(Collectors.toList()));
+}
+
+// Add successful results for empty files (no need to send to SCM)
+Map emptyKeyBlocksList =
keyBlocksList.entrySet().stream()
Review Comment:
Please add a nonEmptyKeyBlocksList and keyBlocksList size check before going
into emptyKeyBlocksList construction and DeleteBlockGroupResult construction.
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
priyeshkaratha commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2707014941
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,21 +139,49 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty keys): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
LOG.info("Send {} key(s) to SCM, first {} keys: {}",
keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
Review Comment:
Thanks @ChenSammi for the catch we can skip empty size keys here. I have
updated the patch.
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ChenSammi commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2706872334
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,21 +139,49 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty keys): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
LOG.info("Send {} key(s) to SCM, first {} keys: {}",
keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
Review Comment:
Shall we include both nonEmptyKeyBlocksList and keyBlocksList info here too?
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ChenSammi commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2706872334
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,21 +139,49 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty keys): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
LOG.info("Send {} key(s) to SCM, first {} keys: {}",
keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
Review Comment:
Shall we use nonEmptyKeyBlocksList here, instead of keyBlocksList?
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
priyeshkaratha commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2691223075
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,23 +139,53 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty files): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
- LOG.info("Send {} key(s) to SCM, first {} keys: {}",
- keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
+ LOG.info("Send {} key(s) to SCM (filtered {} empty files), first {}
keys: {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), logSize,
+ nonEmptyKeyBlocksList.entrySet().stream().limit(logSize)
Review Comment:
Thanks for reviewing the changes. Addressed the changes mentioned.
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
sadanand48 commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2689461970
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,23 +139,53 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty files): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
- LOG.info("Send {} key(s) to SCM, first {} keys: {}",
- keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
+ LOG.info("Send {} key(s) to SCM (filtered {} empty files), first {}
keys: {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), logSize,
+ nonEmptyKeyBlocksList.entrySet().stream().limit(logSize)
Review Comment:
+1. Lets keep them logged as DEBUG. Also probably uniformly use 'keys'
instead of 'files' to maintain uniformity while logging
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ashishkumar50 commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2689430486
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,23 +139,53 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty files): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
- LOG.info("Send {} key(s) to SCM, first {} keys: {}",
- keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
+ LOG.info("Send {} key(s) to SCM (filtered {} empty files), first {}
keys: {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), logSize,
+ nonEmptyKeyBlocksList.entrySet().stream().limit(logSize)
Review Comment:
These empty files can happen mostly in testing, No need to update normal
logs to avoid confusion. Instead please add separate debug log if any empty
files/blocks are present.
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
sadanand48 commented on code in PR #9635:
URL: https://github.com/apache/ozone/pull/9635#discussion_r2689461970
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##
@@ -139,23 +139,53 @@ Pair, Boolean>
processKeyDeletes(Map keyB
String snapTableKey, UUID expectedPreviousSnapshotId) throws IOException
{
long startTime = Time.monotonicNow();
Pair, Boolean> purgeResult = Pair.of(Pair.of(0, 0L),
false);
+
+// Filter out empty files (files with no blocks) before sending to SCM
+Map nonEmptyKeyBlocksList =
keyBlocksList.entrySet().stream()
+.filter(entry -> entry.getValue().getBlockGroup() != null &&
+ entry.getValue().getBlockGroup().getDeletedBlocks()
!= null &&
+
!entry.getValue().getBlockGroup().getDeletedBlocks().isEmpty())
+.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
if (LOG.isDebugEnabled()) {
- LOG.debug("Send {} key(s) to SCM: {}",
- keyBlocksList.size(), keyBlocksList);
+ LOG.debug("Send {} key(s) to SCM (filtered {} empty files): {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), nonEmptyKeyBlocksList);
} else if (LOG.isInfoEnabled()) {
int logSize = 10;
- if (keyBlocksList.size() < logSize) {
-logSize = keyBlocksList.size();
+ if (nonEmptyKeyBlocksList.size() < logSize) {
+logSize = nonEmptyKeyBlocksList.size();
}
- LOG.info("Send {} key(s) to SCM, first {} keys: {}",
- keyBlocksList.size(), logSize,
keyBlocksList.entrySet().stream().limit(logSize)
+ LOG.info("Send {} key(s) to SCM (filtered {} empty files), first {}
keys: {}",
+ nonEmptyKeyBlocksList.size(), keyBlocksList.size() -
nonEmptyKeyBlocksList.size(), logSize,
+ nonEmptyKeyBlocksList.entrySet().stream().limit(logSize)
Review Comment:
+1. Lets keep them logged as DEBUG. Also probably use 'keys' instead of
'files' to maintain uniformity while logging
--
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]
Re: [PR] HDDS-14418. Skip sending block deletion command to SCM for empty files. [ozone]
ChenSammi commented on PR #9635: URL: https://github.com/apache/ozone/pull/9635#issuecomment-3748211911 @priyeshkaratha , can you add a unit test for this case? -- 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]
