You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2025/10/28 15:27:20 UTC
[PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
swamirishi opened a new pull request, #9210:
URL: https://github.com/apache/ozone/pull/9210
## What changes were proposed in this pull request?
A Snapshot Cache lock needs to be implemented which should prevent the rocksdb to be opened while the lock is held.
The idea is, the snapshot can't be opened for other purposes during snapshot defragmentation (note: snapshot compaction is not DB compaction). Essentially, an exclusive lock should be taken on the snapshot during the process.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13004
## How was this patch tested?
Added unit tests
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9210:
URL: https://github.com/apache/ozone/pull/9210#discussion_r2471749045
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -288,14 +288,26 @@ public void release(UUID key) {
*/
public UncheckedAutoCloseableSupplier<OMLockDetails> lock() {
return lock(() -> lock.acquireResourceWriteLock(SNAPSHOT_DB_LOCK),
- () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK));
+ () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK), () -> cleanup(true));
}
- private UncheckedAutoCloseableSupplier<OMLockDetails> lock(
- Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction) {
+ /**
+ * Acquires a write lock on a specific snapshot database and returns an auto-closeble supplier for lock details.
+ * The lock ensures that the operations accessing the snapshot database are perfromed in a thread safe manner. The
+ * returned supplier automatically releases the lock acquired when closed, preveneting potential resource
Review Comment:
typo
```suggestion
* returned supplier automatically releases the lock acquired when closed, preventing potential resource
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #9210:
URL: https://github.com/apache/ozone/pull/9210#discussion_r2471765340
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -324,43 +336,49 @@ public OMLockDetails get() {
* If cache size exceeds soft limit, attempt to clean up and close the
instances that has zero reference count.
Review Comment:
Grammatical error: 'instances that has' should be 'instances that have' for subject-verb agreement.
```suggestion
instances that have zero reference count.
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #9210:
URL: https://github.com/apache/ozone/pull/9210#discussion_r2472170167
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -288,14 +288,26 @@ public void release(UUID key) {
*/
public UncheckedAutoCloseableSupplier<OMLockDetails> lock() {
return lock(() -> lock.acquireResourceWriteLock(SNAPSHOT_DB_LOCK),
- () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK));
+ () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK), () -> cleanup(true));
}
- private UncheckedAutoCloseableSupplier<OMLockDetails> lock(
- Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction) {
+ /**
+ * Acquires a write lock on a specific snapshot database and returns an auto-closeable supplier for lock details.
+ * The lock ensures that the operations accessing the snapshot database are performed in a thread safe manner. The
+ * returned supplier automatically releases the lock acquired when closed, preventing potential resource
+ * contention or deadlocks.
+ */
+ public UncheckedAutoCloseableSupplier<OMLockDetails> lock(UUID snapshotId) {
+ return lock(() -> lock.acquireWriteLock(SNAPSHOT_DB_LOCK, snapshotId.toString()),
+ () -> lock.releaseWriteLock(SNAPSHOT_DB_LOCK, snapshotId.toString()),
+ () -> cleanup(snapshotId));
+ }
+
+ private UncheckedAutoCloseableSupplier<OMLockDetails> lock(Supplier<OMLockDetails> lockFunction,
+ Supplier<OMLockDetails> unlockFunction, Supplier<Void> cleanupFunction) {
AtomicReference<OMLockDetails> lockDetails = new AtomicReference<>(lockFunction.get());
if (lockDetails.get().isLockAcquired()) {
- cleanup(true);
+ cleanupFunction.get();
Review Comment:
why are we closing the snapshot instance upon acquiring a lock?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9210:
URL: https://github.com/apache/ozone/pull/9210#issuecomment-3461087591
thank you @smengcl and @sadanand48 for reviewing 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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9210:
URL: https://github.com/apache/ozone/pull/9210#discussion_r2472646126
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -288,14 +288,26 @@ public void release(UUID key) {
*/
public UncheckedAutoCloseableSupplier<OMLockDetails> lock() {
return lock(() -> lock.acquireResourceWriteLock(SNAPSHOT_DB_LOCK),
- () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK));
+ () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK), () -> cleanup(true));
}
- private UncheckedAutoCloseableSupplier<OMLockDetails> lock(
- Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction) {
+ /**
+ * Acquires a write lock on a specific snapshot database and returns an auto-closeable supplier for lock details.
+ * The lock ensures that the operations accessing the snapshot database are performed in a thread safe manner. The
+ * returned supplier automatically releases the lock acquired when closed, preventing potential resource
+ * contention or deadlocks.
+ */
+ public UncheckedAutoCloseableSupplier<OMLockDetails> lock(UUID snapshotId) {
+ return lock(() -> lock.acquireWriteLock(SNAPSHOT_DB_LOCK, snapshotId.toString()),
+ () -> lock.releaseWriteLock(SNAPSHOT_DB_LOCK, snapshotId.toString()),
+ () -> cleanup(snapshotId));
+ }
+
+ private UncheckedAutoCloseableSupplier<OMLockDetails> lock(Supplier<OMLockDetails> lockFunction,
+ Supplier<OMLockDetails> unlockFunction, Supplier<Void> cleanupFunction) {
AtomicReference<OMLockDetails> lockDetails = new AtomicReference<>(lockFunction.get());
if (lockDetails.get().isLockAcquired()) {
- cleanup(true);
+ cleanupFunction.get();
Review Comment:
We don't want a rocksdb open in the cache once a write lock has been acquired
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9210:
URL: https://github.com/apache/ozone/pull/9210#discussion_r2471753713
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -288,14 +288,26 @@ public void release(UUID key) {
*/
public UncheckedAutoCloseableSupplier<OMLockDetails> lock() {
return lock(() -> lock.acquireResourceWriteLock(SNAPSHOT_DB_LOCK),
- () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK));
+ () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK), () -> cleanup(true));
}
- private UncheckedAutoCloseableSupplier<OMLockDetails> lock(
- Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction) {
+ /**
+ * Acquires a write lock on a specific snapshot database and returns an auto-closeble supplier for lock details.
+ * The lock ensures that the operations accessing the snapshot database are perfromed in a thread safe manner. The
+ * returned supplier automatically releases the lock acquired when closed, preveneting potential resource
Review Comment:
done
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9210:
URL: https://github.com/apache/ozone/pull/9210#discussion_r2471754590
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -288,14 +288,26 @@ public void release(UUID key) {
*/
public UncheckedAutoCloseableSupplier<OMLockDetails> lock() {
return lock(() -> lock.acquireResourceWriteLock(SNAPSHOT_DB_LOCK),
- () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK));
+ () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK), () -> cleanup(true));
}
- private UncheckedAutoCloseableSupplier<OMLockDetails> lock(
- Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction) {
+ /**
+ * Acquires a write lock on a specific snapshot database and returns an auto-closeble supplier for lock details.
+ * The lock ensures that the operations accessing the snapshot database are perfromed in a thread safe manner. The
+ * returned supplier automatically releases the lock acquired when closed, preveneting potential resource
Review Comment:
done
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9210:
URL: https://github.com/apache/ozone/pull/9210#issuecomment-3479368235
@swamirishi please set fix version when resolving Jira issue after PR merge
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #9210:
URL: https://github.com/apache/ozone/pull/9210#discussion_r2471745943
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -288,14 +288,26 @@ public void release(UUID key) {
*/
public UncheckedAutoCloseableSupplier<OMLockDetails> lock() {
return lock(() -> lock.acquireResourceWriteLock(SNAPSHOT_DB_LOCK),
- () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK));
+ () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK), () -> cleanup(true));
}
- private UncheckedAutoCloseableSupplier<OMLockDetails> lock(
- Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction) {
+ /**
+ * Acquires a write lock on a specific snapshot database and returns an auto-closeble supplier for lock details.
+ * The lock ensures that the operations accessing the snapshot database are perfromed in a thread safe manner. The
Review Comment:
Missing hyphen: 'thread safe' should be 'thread-safe'.
```suggestion
* The lock ensures that the operations accessing the snapshot database are perfromed in a thread-safe manner. The
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -288,14 +288,26 @@ public void release(UUID key) {
*/
public UncheckedAutoCloseableSupplier<OMLockDetails> lock() {
return lock(() -> lock.acquireResourceWriteLock(SNAPSHOT_DB_LOCK),
- () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK));
+ () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK), () -> cleanup(true));
}
- private UncheckedAutoCloseableSupplier<OMLockDetails> lock(
- Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction) {
+ /**
+ * Acquires a write lock on a specific snapshot database and returns an auto-closeble supplier for lock details.
+ * The lock ensures that the operations accessing the snapshot database are perfromed in a thread safe manner. The
+ * returned supplier automatically releases the lock acquired when closed, preveneting potential resource
Review Comment:
Multiple spelling errors in the JavaDoc: 'auto-closeble' should be 'auto-closeable', 'perfromed' should be 'performed', 'preveneting' should be 'preventing'.
```suggestion
* Acquires a write lock on a specific snapshot database and returns an auto-closeable supplier for lock details.
* The lock ensures that the operations accessing the snapshot database are performed in a thread safe manner. The
* returned supplier automatically releases the lock acquired when closed, preventing potential resource
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9210:
URL: https://github.com/apache/ozone/pull/9210#issuecomment-3457646689
@SaketaChalamchala can you review this 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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9210:
URL: https://github.com/apache/ozone/pull/9210#discussion_r2472646126
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java:
##########
@@ -288,14 +288,26 @@ public void release(UUID key) {
*/
public UncheckedAutoCloseableSupplier<OMLockDetails> lock() {
return lock(() -> lock.acquireResourceWriteLock(SNAPSHOT_DB_LOCK),
- () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK));
+ () -> lock.releaseResourceWriteLock(SNAPSHOT_DB_LOCK), () -> cleanup(true));
}
- private UncheckedAutoCloseableSupplier<OMLockDetails> lock(
- Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction) {
+ /**
+ * Acquires a write lock on a specific snapshot database and returns an auto-closeable supplier for lock details.
+ * The lock ensures that the operations accessing the snapshot database are performed in a thread safe manner. The
+ * returned supplier automatically releases the lock acquired when closed, preventing potential resource
+ * contention or deadlocks.
+ */
+ public UncheckedAutoCloseableSupplier<OMLockDetails> lock(UUID snapshotId) {
+ return lock(() -> lock.acquireWriteLock(SNAPSHOT_DB_LOCK, snapshotId.toString()),
+ () -> lock.releaseWriteLock(SNAPSHOT_DB_LOCK, snapshotId.toString()),
+ () -> cleanup(snapshotId));
+ }
+
+ private UncheckedAutoCloseableSupplier<OMLockDetails> lock(Supplier<OMLockDetails> lockFunction,
+ Supplier<OMLockDetails> unlockFunction, Supplier<Void> cleanupFunction) {
AtomicReference<OMLockDetails> lockDetails = new AtomicReference<>(lockFunction.get());
if (lockDetails.get().isLockAcquired()) {
- cleanup(true);
+ cleanupFunction.get();
Review Comment:
We don't want a rocksdb open in the cache once a write lock has been acquired. We would be using this for atomically switching the rocksdb directory after defragging a snapshot
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13004. Snapshot Cache lock on a specific snapshotId [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #9210:
URL: https://github.com/apache/ozone/pull/9210
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org