You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "echonesis (via GitHub)" <gi...@apache.org> on 2026/09/23 08:49:52 UTC
[PR] HDDS-13303. [disabled] Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
echonesis opened a new pull request, #11310:
URL: https://github.com/apache/ozone/pull/11310
## What changes were proposed in this pull request?
`TestSnapshotDeletingServiceIntegrationTest` was disabled because its KeyDeletingService(KDS)/SnapshotDeletingService(SDS) lock test depended on background service timing and snapshot metadata flushes. SDS could skip a deletion that had not yet been flushed, while a direct RocksDB update could leave the OM cache with an older snapshot value.
This PR:
- Waits for prior snapshots to become active and for snapshot changes to flush before running the deletion services.
- Keeps the manually updated snapshot metadata consistent between RocksDB and the OM cache.
- Uses test-controlled locks to verify that SDS waits for KDS, and waits for both asynchronous tasks to report failures.
- Removes the `@Unhealthy` and `@Flaky` tags.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13303
## How was this patch tested?
Local Test:
```
mvn -pl :ozone-integration-test test \
-Dtest=TestSnapshotDeletingServiceIntegrationTest \
-DskipShade -DskipRecon -DskipDocs
```
GitHub Actions CI: https://github.com/echonesis/ozone/actions/runs/35828729791
flaky-test-check (10*10 tests): https://github.com/echonesis/ozone/actions/runs/35828816403
--
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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #11310:
URL: https://github.com/apache/ozone/pull/11310#discussion_r4161224958
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingServiceIntegrationTest.java:
##########
@@ -763,9 +774,16 @@ public void testSnapshotDeletingServiceWaitsForKeyDeletingService(boolean kdsRun
kds.shutdown();
KeyDeletingService.KeyDeletingTask task = kds.new KeyDeletingTask(snap3Id);
- CompletableFuture.supplyAsync(() -> {
- try (MockedConstruction<ReclaimableKeyFilter> mockedReclaimableFilter = getMockedReclaimableKeyFilter(
- volume, bucket, kdsWaitStarted, sdsLockWaitStarted, sdsLockAcquired, kdsFinished, keyFilter)) {
+ CompletableFuture<?> kdsFuture = CompletableFuture.supplyAsync(() -> {
+ try (UncheckedAutoCloseableSupplier<OmSnapshot> currentSnapshot = kdsRunningOnAOS ? null :
Review Comment:
this is too unreadable.
--
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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #11310:
URL: https://github.com/apache/ozone/pull/11310#discussion_r4099808899
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingServiceIntegrationTest.java:
##########
@@ -624,35 +626,39 @@ private synchronized void createSnapshotDataForBucket(OzoneBucket bucket) throws
bucket.getName()));
}
- private MockedConstruction<ReclaimableKeyFilter> getMockedReclaimableKeyFilter(String volume, String bucket,
+ private MockedConstruction<ReclaimableKeyFilter> getMockedReclaimableKeyFilter(OzoneBucket bucket,
AtomicBoolean kdsWaitStarted, AtomicBoolean sdsLockWaitStarted,
- AtomicBoolean sdsLockAcquired, AtomicBoolean kdsFinished, ReclaimableKeyFilter keyFilter) throws IOException {
+ AtomicBoolean sdsLockAcquired, AtomicBoolean kdsFinished, MultiSnapshotLocks kdsMultiLocks,
+ List<UUID> expectedIds) {
return mockConstruction(ReclaimableKeyFilter.class,
(mocked, context) -> {
when(mocked.apply(any())).thenAnswer(i -> {
Table.KeyValue<String, OmKeyInfo> keyInfo = i.getArgument(0);
- if (!keyInfo.getValue().getVolumeName().equals(volume) ||
- !keyInfo.getValue().getBucketName().equals(bucket)) {
- return keyFilter.apply(i.getArgument(0));
+ if (!keyInfo.getValue().getVolumeName().equals(bucket.getVolumeName()) ||
+ !keyInfo.getValue().getBucketName().equals(bucket.getName())) {
+ return false;
}
- keyFilter.apply(i.getArgument(0));
+ if (kdsWaitStarted.get()) {
+ return true;
+ }
+ assertTrue(kdsMultiLocks.acquireLock(expectedIds).isLockAcquired());
Review Comment:
The mocked apply() now acquires a separate test lock and returns true, so KDS never runs ReclaimableFilter.apply() or acquires its production snapshot GC read lock. The test can pass if that production locking breaks. true can also mark a key retained by a snapshot for deletion.
Please delegate to a real ReclaimableKeyFilter and make the SDS test lock use the OM metadata manager’s lock. The new flush and cache waits can stay.
--
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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #11310:
URL: https://github.com/apache/ozone/pull/11310#discussion_r4099810390
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingServiceIntegrationTest.java:
##########
@@ -624,35 +626,39 @@ private synchronized void createSnapshotDataForBucket(OzoneBucket bucket) throws
bucket.getName()));
}
- private MockedConstruction<ReclaimableKeyFilter> getMockedReclaimableKeyFilter(String volume, String bucket,
+ private MockedConstruction<ReclaimableKeyFilter> getMockedReclaimableKeyFilter(OzoneBucket bucket,
AtomicBoolean kdsWaitStarted, AtomicBoolean sdsLockWaitStarted,
- AtomicBoolean sdsLockAcquired, AtomicBoolean kdsFinished, ReclaimableKeyFilter keyFilter) throws IOException {
+ AtomicBoolean sdsLockAcquired, AtomicBoolean kdsFinished, MultiSnapshotLocks kdsMultiLocks,
+ List<UUID> expectedIds) {
return mockConstruction(ReclaimableKeyFilter.class,
(mocked, context) -> {
when(mocked.apply(any())).thenAnswer(i -> {
Table.KeyValue<String, OmKeyInfo> keyInfo = i.getArgument(0);
- if (!keyInfo.getValue().getVolumeName().equals(volume) ||
- !keyInfo.getValue().getBucketName().equals(bucket)) {
- return keyFilter.apply(i.getArgument(0));
+ if (!keyInfo.getValue().getVolumeName().equals(bucket.getVolumeName()) ||
+ !keyInfo.getValue().getBucketName().equals(bucket.getName())) {
+ return false;
}
- keyFilter.apply(i.getArgument(0));
+ if (kdsWaitStarted.get()) {
+ return true;
+ }
+ assertTrue(kdsMultiLocks.acquireLock(expectedIds).isLockAcquired());
Review Comment:
```diff
@@
- AtomicBoolean sdsLockAcquired, AtomicBoolean kdsFinished, MultiSnapshotLocks kdsMultiLocks,
- List<UUID> expectedIds) {
+ AtomicBoolean sdsLockAcquired, AtomicBoolean kdsFinished, ReclaimableKeyFilter keyFilter) {
@@
- return false;
+ return keyFilter.apply(i.getArgument(0));
}
- if (kdsWaitStarted.get()) {
- return true;
- }
- assertTrue(kdsMultiLocks.acquireLock(expectedIds).isLockAcquired());
+ boolean reclaimable = keyFilter.apply(i.getArgument(0));
@@
- return true;
+ return reclaimable;
@@
- kdsMultiLocks.releaseLock();
+ keyFilter.close();
@@
- when(mocked.getExclusiveReplicatedSizeMap()).thenReturn(Collections.emptyMap());
- when(mocked.getExclusiveSizeMap()).thenReturn(Collections.emptyMap());
+ when(mocked.getExclusiveReplicatedSizeMap()).thenAnswer(i -> keyFilter.getExclusiveReplicatedSizeMap());
+ when(mocked.getExclusiveSizeMap()).thenAnswer(i -> keyFilter.getExclusiveSizeMap());
@@
- IOzoneManagerLock snapshotGcLock = new OzoneManagerLock(new OzoneConfiguration());
@@
- MultiSnapshotLocks kdsMultiLocks = new MultiSnapshotLocks(snapshotGcLock, SNAPSHOT_GC_LOCK, false);
- MultiSnapshotLocks sdsMultiLocks = new MultiSnapshotLocks(snapshotGcLock, SNAPSHOT_GC_LOCK, true);
+ SnapshotInfo snapInfo = kdsRunningOnAOS ? null : SnapshotUtils.getSnapshotInfo(om, volume, bucket, "snap2");
+ ReclaimableKeyFilter keyFilter = new ReclaimableKeyFilter(om, om.getOmSnapshotManager(),
+ snapshotChainManager, snapInfo,
+ kdsRunningOnAOS ? om.getKeyManager() : getOmSnapshot(volume, bucket, "snap2").get().getKeyManager(),
+ om.getMetadataManager().getLock());
+ MultiSnapshotLocks sdsMultiLocks =
+ new MultiSnapshotLocks(om.getMetadataManager().getLock(), SNAPSHOT_GC_LOCK, true);
@@
- ozoneBucket, kdsWaitStarted, sdsLockWaitStarted, sdsLockAcquired, kdsFinished, kdsMultiLocks,
- expectedIds)) {
+ ozoneBucket, kdsWaitStarted, sdsLockWaitStarted, sdsLockAcquired, kdsFinished, keyFilter)) {
```
and remove unused imports
--
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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "echonesis (via GitHub)" <gi...@apache.org>.
echonesis commented on PR #11310:
URL: https://github.com/apache/ozone/pull/11310#issuecomment-5963218936
Thanks @smengcl , @rich7420 for the review, @jojochuang for the review and 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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "echonesis (via GitHub)" <gi...@apache.org>.
echonesis commented on code in PR #11310:
URL: https://github.com/apache/ozone/pull/11310#discussion_r4163310809
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingServiceIntegrationTest.java:
##########
@@ -763,9 +774,16 @@ public void testSnapshotDeletingServiceWaitsForKeyDeletingService(boolean kdsRun
kds.shutdown();
KeyDeletingService.KeyDeletingTask task = kds.new KeyDeletingTask(snap3Id);
- CompletableFuture.supplyAsync(() -> {
- try (MockedConstruction<ReclaimableKeyFilter> mockedReclaimableFilter = getMockedReclaimableKeyFilter(
- volume, bucket, kdsWaitStarted, sdsLockWaitStarted, sdsLockAcquired, kdsFinished, keyFilter)) {
+ CompletableFuture<?> kdsFuture = CompletableFuture.supplyAsync(() -> {
+ try (UncheckedAutoCloseableSupplier<OmSnapshot> currentSnapshot = kdsRunningOnAOS ? null :
Review Comment:
Thanks @jojochuang for the feedback!
I've split the resource setup into two try blocks and extracted the KeyManager selection to make it easier to read.
--
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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #11310:
URL: https://github.com/apache/ozone/pull/11310#issuecomment-5959842199
Merged. The failed test is unrelated. @echonesis
--
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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #11310:
URL: https://github.com/apache/ozone/pull/11310
--
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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #11310:
URL: https://github.com/apache/ozone/pull/11310#discussion_r4163340385
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingServiceIntegrationTest.java:
##########
@@ -763,9 +774,16 @@ public void testSnapshotDeletingServiceWaitsForKeyDeletingService(boolean kdsRun
kds.shutdown();
KeyDeletingService.KeyDeletingTask task = kds.new KeyDeletingTask(snap3Id);
- CompletableFuture.supplyAsync(() -> {
- try (MockedConstruction<ReclaimableKeyFilter> mockedReclaimableFilter = getMockedReclaimableKeyFilter(
- volume, bucket, kdsWaitStarted, sdsLockWaitStarted, sdsLockAcquired, kdsFinished, keyFilter)) {
+ CompletableFuture<?> kdsFuture = CompletableFuture.supplyAsync(() -> {
+ try (UncheckedAutoCloseableSupplier<OmSnapshot> currentSnapshot = kdsRunningOnAOS ? null :
Review Comment:
it's a fine attempt but it's not helping. I am looking for something like this:
```
try (UncheckedAutoCloseableSupplier<OmSnapshot> currentSnapshot =
openCurrentSnapshot(kdsRunningOnAOS, volume, bucket);
ReclaimableKeyFilter keyFilter =
createReclaimableKeyFilter(kdsRunningOnAOS, snapInfo, currentSnapshot)) {
```
--
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-13303. Fix TestSnapshotDeletingServiceIntegrationTest [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #11310:
URL: https://github.com/apache/ozone/pull/11310#issuecomment-5955197581
+1 looks much better.
--
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