HDDS-9107. Reduce the granularity of Container locks for BlockDeletingService #5149
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kerneltime
reviewed
Aug 15, 2023
| LOG.info("Max lock hold time ({} ms) reached after {} ms. " + | ||
| "Completed {} transactions, deleted {} blocks." + | ||
| "Releasing lock and resuming deletion later.", | ||
| getBlockDeletingMaxLockHoldingTime().toMillis(), |
There was a problem hiding this comment.
Log the container information as well.
ChenSammi
reviewed
Sep 8, 2023
| ContainerBackgroundTaskResult crr = handleDeleteTask(); | ||
| blocksToDelete -= crr.getSize(); | ||
| result.addAll(crr.getDeletedBlocks()); | ||
| if (blocksToDelete > 0 && crr.getSize() == 0) { |
There was a problem hiding this comment.
Please move this check ahead, follow the handleDeleteTask call.
ChenSammi
reviewed
Sep 8, 2023
| @@ -167,6 +192,7 @@ public ContainerBackgroundTaskResult deleteViaSchema1( | |||
| if (toDeleteBlocks.isEmpty()) { | |||
| LOG.debug("No under deletion block found in container : {}", | |||
| containerData.getContainerID()); | |||
| blocksToDelete = 0; | |||
There was a problem hiding this comment.
The reset is not necessary, can be removed.
ChenSammi
reviewed
Sep 8, 2023
| @@ -321,6 +347,7 @@ private ContainerBackgroundTaskResult deleteViaTransactionStore( | |||
| // actually no delete transactions for the container, so reset the | |||
| // pending delete block count to the correct value of zero. | |||
| containerData.resetPendingDeleteBlockCount(meta); | |||
| blocksToDelete = 0; | |||
ChenSammi
reviewed
Sep 8, 2023
| @@ -335,12 +362,18 @@ private ContainerBackgroundTaskResult deleteViaTransactionStore( | |||
| int deletedBlocksProcessed = deleteBlocksResult.getBlocksProcessed(); | |||
| int deletedBlocksCount = deleteBlocksResult.getBlocksDeleted(); | |||
| long releasedBytes = deleteBlocksResult.getBytesReleased(); | |||
| List<DeletedBlocksTransaction> deletedBlocksTxs = | |||
| deleteBlocksResult.getDeletedBlockTransactions(); | |||
| deleteBlocksResult.getDeletedBlockTransactions().forEach( | |||
There was a problem hiding this comment.
deleteBlocksResult.getDeletedBlockTransactions() -> deletedBlocksTxs
|
@xichen01 , can you add a unit test in TestBlockDeletingService.java for this if possible? |
I have added a unit test in TestBlockDeletingService.java |
ChenSammi
approved these changes
Sep 13, 2023
errose28
added a commit
to errose28/ozone
that referenced
this pull request
Sep 14, 2023
* master: (55 commits) HDDS-9236. Fix snapdiff output for key modification (apache#5258) HDDS-8013. Freon S3 bucket creation test should use unique prefix (apache#5282) HDDS-9228. Poor S3G read performance (apache#5274) HDDS-8941. Disable flaky TestContainerBalancerTask#testDelayedStart HDDS-1159. Remove flaky tag from TestContainerStateManagerIntegration (apache#5291) HDDS-6077. Remove flaky tag from TestAddRemoveOzoneManager (apache#5290) HDDS-6610. Remove support for recursive volume list/delete using ozone fs command (apache#5264) HDDS-7752. GetS3SecretRequest API should not return secret if secret of user already exists (apache#4538) HDDS-9173. Invalidate snapshot cache once snapshot gets purged (apache#5248) HDDS-8920. Ozone is supporting unicode volume and bucket names, unintentionally (apache#5276) HDDS-9275. LegacyReplicationManager: Delete excess unhealthy with force=true (apache#5286) HDDS-9264. Execute EC acceptance test in secure environment (apache#5279) HDDS-9161. Recon Pipelines datanode columns search does not work (apache#5213) HDDS-9107. Reduce the granularity of Container locks for BlockDeletingService (apache#5149) HDDS-9270. Create a script to list all acceptance test splits (apache#5281) HDDS-9220. Let ContainerBalancerConfiguration#toString print more info (apache#5228) HDDS-9208. Add queue limit in ReplicationServer. (apache#5216) HDDS-9268. [Snapshot] Update list of snapshot apis to include lsDiff details in docs. (apache#5278) HDDS-9234. OM should shutdown immediately if certificate durations are invalid (apache#5243) HDDS-9136. Throw exception when rename fails during moveToTrash. (apache#5253) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Reduce the granularity of Container locks for BlockDeletingService, Reduces the amount of time the BlockDeletingService holds a
Containerlock to reduce affect to others threads who need acquire theContainerlockWhat is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9107
How was this patch tested?
existing test