Skip to content

HDDS-13640. Add CLI that allows manually triggering snapshot defrag#9155

Merged
swamirishi merged 19 commits intoapache:masterfrom
smengcl:HDDS-13640-trigger
Nov 5, 2025
Merged

HDDS-13640. Add CLI that allows manually triggering snapshot defrag#9155
swamirishi merged 19 commits intoapache:masterfrom
smengcl:HDDS-13640-trigger

Conversation

@smengcl
Copy link
Contributor

@smengcl smengcl commented Oct 15, 2025

What changes were proposed in this pull request?

Add a CLI subcommand that allows snapshot defrag service to be manually triggered.

ozone admin om snapshot defrag --service-id=omservice --node-id=om3

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13640

How was this patch tested?

  • Added unit test
  • Added integration test
  • Manual testing / logs

@smengcl smengcl requested review from Copilot and swamirishi October 15, 2025 07:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a CLI command to manually trigger the snapshot defragmentation service in Ozone Manager, allowing administrators to initiate snapshot defrag operations on demand.

Key changes:

  • Added new protobuf message types for snapshot defrag requests/responses
  • Implemented the snapshot defrag trigger functionality in OzoneManager with admin privilege checks
  • Created a new CLI subcommand triggerSnapshotDefrag with options for service ID, host, and no-wait execution

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
OmClientProtocol.proto Added SnapshotDefrag message types and enum value
OzoneManagerProtocol.java Added triggerSnapshotDefrag method to protocol interface
OzoneManager.java Implemented snapshot defrag trigger with security checks and async execution
OzoneManagerRequestHandler.java Added request handling for SnapshotDefrag type
OzoneManagerProtocolClientSideTranslatorPB.java Added client-side implementation for snapshot defrag requests
OmUtils.java Added SnapshotDefrag to read-only operation types
TriggerSnapshotDefragSubCommand.java New CLI command implementation
OMAdmin.java Registered new subcommand with OM admin CLI

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@errose28
Copy link
Contributor

ozone admin om triggerSnapshotDefrag -id=ozone1

A better CLI name would probably be ozone admin om snapshot defrag. trigger is redundant since we are already executing a command. This will likely not be the last snapshot admin operation added so we should create a separate subcommand to group them.

@jojochuang jojochuang added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Oct 20, 2025
@smengcl
Copy link
Contributor Author

smengcl commented Nov 1, 2025

ozone admin om triggerSnapshotDefrag -id=ozone1

A better CLI name would probably be ozone admin om snapshot defrag. trigger is redundant since we are already executing a command. This will likely not be the last snapshot admin operation added so we should create a separate subcommand to group them.

Good idea! Done

@smengcl smengcl requested a review from Copilot November 1, 2025 07:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor issues but overall good to me. I'd suggest to merge it and fix the rest in a follow up task.

@CommandLine.Option(
names = {"--node-id"},
description = "NodeID of the OM to trigger snapshot defragmentation on.",
required = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is not required, which OM node is going to be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be specified, with the current framework of how OMNodeDetails is retrieved. It won't select OM leader as I initially expected. So I added that null check.

@jojochuang jojochuang marked this pull request as ready for review November 2, 2025 04:29
…ozone/om/snapshot/TestSnapshotDefragAdmin.java

Co-authored-by: Wei-Chiu Chuang <jojochuang@gmail.com>
Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@swamirishi swamirishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@swamirishi swamirishi merged commit af123a5 into apache:master Nov 5, 2025
44 checks passed
@swamirishi
Copy link
Contributor

thanks for the patch @smengcl . Thank you @jojochuang for reviewing the patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

snapshot https://issues.apache.org/jira/browse/HDDS-6517

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants