Skip to content

HDDS-14076. Disable SSTFilteringService when defrag feature is enabled#9495

Merged
smengcl merged 4 commits intoapache:masterfrom
smengcl:HDDS-14076-disable-sfs-on-defrag-enabled
Dec 19, 2025
Merged

HDDS-14076. Disable SSTFilteringService when defrag feature is enabled#9495
smengcl merged 4 commits intoapache:masterfrom
smengcl:HDDS-14076-disable-sfs-on-defrag-enabled

Conversation

@smengcl
Copy link
Copy Markdown
Contributor

@smengcl smengcl commented Dec 13, 2025

What changes were proposed in this pull request?

  1. Disable SSTFilteringService when defrag feature is enabled, because defrag is effectively doing the filtering already
  2. SSTFilteringService should not run on defragged snapshots

What is the link to the Apache JIRA

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

How was this patch tested?

  • Added UT

@smengcl smengcl added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Dec 13, 2025
@smengcl smengcl requested a review from swamirishi December 13, 2025 04:02
Copy link
Copy Markdown
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.

looks good. i'll let @swamirishi to take another look

Copy link
Copy Markdown
Contributor

@sadanand48 sadanand48 left a comment

Choose a reason for hiding this comment

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

Thanks @smengcl for the patch. LGTM

@smengcl smengcl marked this pull request as ready for review December 18, 2025 19:07
Copilot AI review requested due to automatic review settings December 18, 2025 19:07
Copy link
Copy Markdown
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 optimizes snapshot SST filtering by disabling the SSTFilteringService when the defrag service is enabled, since defragmentation already performs filtering operations. It also prevents filtering from running on snapshots that have already been defragged.

Key changes:

  • Added logic to disable SSTFilteringService initialization when SnapshotDefragService is enabled
  • Enhanced SSTFilteringService to skip defragged snapshots during processing
  • Added a new method to expose the last defrag timestamp from snapshot local data

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
KeyManagerImpl.java Added check to prevent SSTFilteringService from starting when defrag service is enabled
SstFilteringService.java Added logic to skip defragged snapshots during filtering, preventing redundant work
OmSnapshotLocalDataManager.java Added getter method to expose last defrag time for determining if a snapshot has been defragged
TestSstFilteringService.java Added unit test to verify SSTFilteringService is properly disabled when defrag service is enabled

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

Copy link
Copy Markdown
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.

+1 pending CI

Copy link
Copy Markdown
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.

@smengcl thanks for the patch. Changes look good to me apart from the minor comment I have left.

@smengcl
Copy link
Copy Markdown
Contributor Author

smengcl commented Dec 19, 2025

Thanks @sadanand48 @jojochuang @swamirishi for the reviews.

@smengcl smengcl merged commit 1dda9ab into apache:master Dec 19, 2025
43 checks passed
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