Skip to content

HDDS-7741. [Snapshot] Delete keys in snapshot scope from deleteTable during createSnapshot to accommodate snapshot garbage collection #4280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
smengcl merged 11 commits into from
Mar 16, 2023
Merged

HDDS-7741. [Snapshot] Delete keys in snapshot scope from deleteTable during createSnapshot to accommodate snapshot garbage collection #4280

smengcl merged 11 commits into from
Mar 16, 2023

Conversation

@smengcl
Copy link
Contributor

@smengcl smengcl commented Feb 15, 2023 •
edited
Loading

What changes were proposed in this pull request?

During snapshot creation, use deleteRange to remove the keys in the snapshot scope (bucket) from deletedTable.

  • Implement deleteRange in Table class to expose RocksDB deleteRange API.
  • Add UT for deleteRange
  • Add table lock for deletedTable
  • UT addition to test the new logic addition in OMSnapshotCreateRequest#validateAndUpdateCache
  • Adjust deletedTable clean up logic location according to HDDS-7906
  • Add TODOs for upcoming tasks

What is the link to the Apache JIRA

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

How was this patch tested?

  • Added new UTs.

@smengcl
HDDS-7741. [Snapshot] Delete keys in snapshot scope from deleteTable …
753d5c6 
…during createSnapshot to accommodate snapshot garbage collection
@smengcl smengcl added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Feb 15, 2023
@smengcl
Clean up
080d946 
Change-Id: I2672cbd4d92ef12c4c0e0d678d0485c55f6892d0
@smengcl
Merge remote-tracking branch 'asf/master' into HDDS-7741
4d8010c 
Change-Id: Ibbf5f8c3ed29f4d235af4c75d40c351b55a3cc98
@smengcl
Move deleteRange from deletedTable logic to OmSnapshotManager
97db4b5 
Change-Id: If40fe3b6397c0f53c5970bd17a0b3ecbadacf2d8
@smengcl
Adjust unit tests.
444187b 
Change-Id: Ide13aee59fc84d785aba5bc1ff735529c48149e3
@smengcl smengcl marked this pull request as ready for review February 22, 2023 20:14
@smengcl
Clean up
3fe55d1 
Change-Id: I5acfd992ae38f48f73c23d137d83b40c1cc79b8a
@smengcl
Mention HDDS-7968 in TODO comment block.
b540f9e 
Change-Id: I478aaff0d112f8f14381dd04aac994afc446e70a
@smengcl
Remove unused method wrapper
d963d8b 
Change-Id: Ida129a5821dedfc5883868ec1597bad014317662
@smengcl smengcl mentioned this pull request Mar 2, 2023
Merged
@GeorgeJahad GeorgeJahad mentioned this pull request Mar 7, 2023
Merged
smengcl added 2 commits March 8, 2023 09:47
@smengcl
Add perf timer; more TODOs.
ee2836e 
Change-Id: I8f8dab1473b3e7c65acd2e04ef0ed13367d15082
@smengcl
Merge remote-tracking branch 'asf/master' into HDDS-7741
fcd39df 
Conflicts:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotCreateResponse.java

Change-Id: I7c1bcce86d7a98cd83aa3b63be446ab94a501102
prashantpogde
prashantpogde approved these changes Mar 10, 2023
View reviewed changes
Copy link
Contributor

@prashantpogde prashantpogde 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 making these changes. LGTM

@smengcl
Mention the table lock is not intended to be used widely. Would be re…
06e0285 
…moved when proper refactor HDDS-5905 (HDDS-6483) is in.

Change-Id: I74e5cb7a8a147a78f61dcfc6325fe6ec1f96b517
@smengcl
Copy link
Contributor Author

smengcl commented Mar 15, 2023

Thanks @prashantpogde for the review.

I've added a note that the lock is a stopgap solution before the proper fix (HDDS-5905, HDDS-6483) is in. As I talked to @kerneltime last week that it is not ideal to have a lock on a RocksDB table in the first place.

@smengcl smengcl merged commit c7de8d5 into apache:master Mar 16, 2023
@smengcl
Copy link
Contributor Author

smengcl commented Mar 16, 2023

Thanks @prashantpogde for the review.

@aswinshakil aswinshakil mentioned this pull request Mar 30, 2023
Merged
@smengcl smengcl mentioned this pull request May 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GeorgeJahad GeorgeJahad GeorgeJahad left review comments

@prashantpogde prashantpogde prashantpogde approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@smengcl @GeorgeJahad @prashantpogde