From 80365fbb3751a535eb0affd90dcfe1553a972c10 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Thu, 29 Oct 2026 03:20:45 +0800 Subject: [PATCH 1/2] HDDS-17240. Repair old volume quota without losing unflushed volume changes The repair found legacy volumes through the RocksDB iterator and put the row it read back into the fully cached volume table. A volume delete, a bucket count or tenant refCount change applied just before it but not yet flushed was rolled back, and a volume created but not yet flushed kept its old quota, so OMs diverged depending on how far their double buffer had flushed. Its response also wrote each volume under the bare name, so the conversion was lost on restart and the volume was listed twice. Both are fixed together: correcting only the key would make the rolled back value durable. --- .../request/volume/OMQuotaRepairRequest.java | 70 ++++++++++--------- .../volume/OMQuotaRepairResponse.java | 3 +- .../volume/TestOMQuotaRepairRequest.java | 57 +++++++++++++++ .../ozone/om/service/TestQuotaRepairTask.java | 5 ++ 4 files changed, 101 insertions(+), 34 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMQuotaRepairRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMQuotaRepairRequest.java index 3094de9eb343..bf75b321dc8c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMQuotaRepairRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMQuotaRepairRequest.java @@ -25,11 +25,10 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; import java.util.Objects; import org.apache.commons.lang3.tuple.Pair; -import org.apache.hadoop.hdds.utils.db.Table; -import org.apache.hadoop.hdds.utils.db.TableIterator; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.om.OMMetadataManager; @@ -174,39 +173,44 @@ private Map updateOldVolumeQuotaSupport( OMMetadataManager metadataManager, long transactionLogIndex) throws IOException { LOG.info("Starting volume quota support update"); Map volUpdateMap = new HashMap<>(); - try (TableIterator> - iterator = metadataManager.getVolumeTable().iterator()) { - while (iterator.hasNext()) { - Table.KeyValue entry = iterator.next(); - OmVolumeArgs omVolumeArgs = entry.getValue(); - if (!(omVolumeArgs.getQuotaInBytes() == OLD_QUOTA_DEFAULT - || omVolumeArgs.getQuotaInNamespace() == OLD_QUOTA_DEFAULT)) { + // The volume table is fully cached; iterate the cache to see volume changes not flushed yet. + Iterator, CacheValue>> iterator = + metadataManager.getVolumeTable().cacheIterator(); + while (iterator.hasNext()) { + Map.Entry, CacheValue> entry = iterator.next(); + String volumeKey = entry.getKey().getCacheKey(); + OmVolumeArgs omVolumeArgs = entry.getValue().getCacheValue(); + if (omVolumeArgs == null || !(omVolumeArgs.getQuotaInBytes() == OLD_QUOTA_DEFAULT + || omVolumeArgs.getQuotaInNamespace() == OLD_QUOTA_DEFAULT)) { + continue; + } + mergeOmLockDetails(metadataManager.getLock().acquireWriteLock( + VOLUME_LOCK, omVolumeArgs.getVolume())); + boolean acquiredVolumeLock = getOmLockDetails().isLockAcquired(); + try { + OmVolumeArgs currentVolumeArgs = metadataManager.getVolumeTable().get(volumeKey); + if (currentVolumeArgs == null) { continue; } - mergeOmLockDetails(metadataManager.getLock().acquireWriteLock( - VOLUME_LOCK, omVolumeArgs.getVolume())); - boolean acquiredVolumeLock = getOmLockDetails().isLockAcquired(); - try { - OmVolumeArgs.Builder builder = omVolumeArgs.toBuilder(); - boolean isQuotaReset = false; - if (omVolumeArgs.getQuotaInBytes() == OLD_QUOTA_DEFAULT) { - builder.setQuotaInBytes(QUOTA_RESET); - isQuotaReset = true; - } - if (omVolumeArgs.getQuotaInNamespace() == OLD_QUOTA_DEFAULT) { - builder.setQuotaInNamespace(QUOTA_RESET); - isQuotaReset = true; - } - if (isQuotaReset) { - OmVolumeArgs updated = builder.build(); - metadataManager.getVolumeTable().addCacheEntry( - new CacheKey<>(entry.getKey()), CacheValue.get(transactionLogIndex, updated)); - volUpdateMap.put(entry.getKey(), updated); - } - } finally { - if (acquiredVolumeLock) { - mergeOmLockDetails(metadataManager.getLock().releaseWriteLock(VOLUME_LOCK, omVolumeArgs.getVolume())); - } + OmVolumeArgs.Builder builder = currentVolumeArgs.toBuilder(); + boolean isQuotaReset = false; + if (currentVolumeArgs.getQuotaInBytes() == OLD_QUOTA_DEFAULT) { + builder.setQuotaInBytes(QUOTA_RESET); + isQuotaReset = true; + } + if (currentVolumeArgs.getQuotaInNamespace() == OLD_QUOTA_DEFAULT) { + builder.setQuotaInNamespace(QUOTA_RESET); + isQuotaReset = true; + } + if (isQuotaReset) { + OmVolumeArgs updated = builder.build(); + metadataManager.getVolumeTable().addCacheEntry( + new CacheKey<>(volumeKey), CacheValue.get(transactionLogIndex, updated)); + volUpdateMap.put(volumeKey, updated); + } + } finally { + if (acquiredVolumeLock) { + mergeOmLockDetails(metadataManager.getLock().releaseWriteLock(VOLUME_LOCK, omVolumeArgs.getVolume())); } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/volume/OMQuotaRepairResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/volume/OMQuotaRepairResponse.java index 7f913c61d2d1..9970f1c6f9f9 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/volume/OMQuotaRepairResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/volume/OMQuotaRepairResponse.java @@ -60,7 +60,8 @@ public void addToDBBatch(OMMetadataManager metadataManager, omBucketInfo.getBucketName()), omBucketInfo); } for (OmVolumeArgs volArgs : volumeArgsMap.values()) { - metadataManager.getVolumeTable().putWithBatch(batchOp, volArgs.getVolume(), volArgs); + metadataManager.getVolumeTable().putWithBatch(batchOp, + metadataManager.getVolumeKey(volArgs.getVolume()), volArgs); } } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMQuotaRepairRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMQuotaRepairRequest.java index 49b3213b30c0..39b53949a25c 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMQuotaRepairRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMQuotaRepairRequest.java @@ -17,14 +17,18 @@ package org.apache.hadoop.ozone.om.request.volume; +import static org.apache.hadoop.ozone.OzoneConsts.OLD_QUOTA_DEFAULT; +import static org.apache.hadoop.ozone.OzoneConsts.QUOTA_RESET; import static org.assertj.core.api.Assertions.assertThat; import java.util.UUID; import org.apache.hadoop.hdds.utils.db.BatchOperation; +import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.om.request.key.OMKeyRequest; import org.apache.hadoop.ozone.om.response.OMClientResponse; @@ -147,4 +151,57 @@ public void testStaleDeltaSkippedWhenBucketRecreatedUnderSameName() throws Excep assertThat(cachedBucket.getUsedBytes()).isEqualTo(0); assertThat(cachedBucket.getUsedNamespace()).isEqualTo(0); } + + @Test + public void testOldVolumeQuotaRepairKeepsUnflushedVolumeChanges() throws Exception { + String deletedVolumeKey = addOldQuotaVolume(true); + String usedVolumeKey = addOldQuotaVolume(true); + String quotaSetVolumeKey = addOldQuotaVolume(true); + String unchangedVolumeKey = addOldQuotaVolume(true); + String createdVolumeKey = addOldQuotaVolume(false); + + // Volume changes applied after those rows were flushed, still waiting for the next flush. + Table volumeTable = omMetadataManager.getVolumeTable(); + volumeTable.addCacheEntry(new CacheKey<>(deletedVolumeKey), CacheValue.get(2L)); + volumeTable.addCacheEntry(new CacheKey<>(usedVolumeKey), CacheValue.get(3L, + volumeTable.get(usedVolumeKey).toBuilder().setUsedNamespace(1).incRefCount().build())); + volumeTable.addCacheEntry(new CacheKey<>(quotaSetVolumeKey), CacheValue.get(4L, + volumeTable.get(quotaSetVolumeKey).toBuilder().setQuotaInNamespace(10).build())); + + OMRequest omRequest = OMRequest.newBuilder() + .setClientId("test-client") + .setCmdType(Type.QuotaRepair) + .setQuotaRepairRequest(QuotaRepairRequest.newBuilder().setSupportVolumeOldQuota(true).build()) + .build(); + OMClientResponse omClientResponse = new OMQuotaRepairRequest(omRequest).validateAndUpdateCache(ozoneManager, 5L); + assertThat(omClientResponse.getOMResponse().getStatus()).isEqualTo(Status.OK); + + assertThat(volumeTable.get(deletedVolumeKey)).as("volume deleted before the repair").isNull(); + OmVolumeArgs usedVolume = volumeTable.get(usedVolumeKey); + assertThat(usedVolume.getQuotaInNamespace()).isEqualTo(QUOTA_RESET); + assertThat(usedVolume.getUsedNamespace()).isEqualTo(1); + assertThat(usedVolume.getRefCount()).isEqualTo(1); + assertThat(volumeTable.get(quotaSetVolumeKey).getQuotaInNamespace()).isEqualTo(10); + assertThat(volumeTable.get(unchangedVolumeKey).getQuotaInNamespace()).isEqualTo(QUOTA_RESET); + assertThat(volumeTable.get(createdVolumeKey).getQuotaInNamespace()).as("volume created before the repair") + .isEqualTo(QUOTA_RESET); + } + + /** Adds a volume as written by a release without namespace quota to the table cache, and to RocksDB if flushed. */ + private String addOldQuotaVolume(boolean flushed) throws Exception { + String volumeName = UUID.randomUUID().toString(); + OmVolumeArgs volumeArgs = OmVolumeArgs.newBuilder() + .setVolume(volumeName) + .setAdminName("admin") + .setOwnerName("owner") + .setCreationTime(Time.now()) + .setQuotaInNamespace(OLD_QUOTA_DEFAULT) + .build(); + String volumeKey = omMetadataManager.getVolumeKey(volumeName); + if (flushed) { + omMetadataManager.getVolumeTable().put(volumeKey, volumeArgs); + } + omMetadataManager.getVolumeTable().addCacheEntry(new CacheKey<>(volumeKey), CacheValue.get(1L, volumeArgs)); + return volumeKey; + } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestQuotaRepairTask.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestQuotaRepairTask.java index 3f47ed06bd93..6ca4deb016b9 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestQuotaRepairTask.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestQuotaRepairTask.java @@ -231,6 +231,11 @@ public void testQuotaRepairForOldVersionVolumeBucket() throws Exception { .get(omMetadataManager.getVolumeKey(volumeName)); assertEquals(-1, volArgsVerify.getQuotaInBytes()); assertEquals(-1, volArgsVerify.getQuotaInNamespace()); + OmVolumeArgs persistedVolArgs = omMetadataManager.getVolumeTable() + .getSkipCache(omMetadataManager.getVolumeKey(volumeName)); + assertEquals(-1, persistedVolArgs.getQuotaInBytes()); + assertEquals(-1, persistedVolArgs.getQuotaInNamespace()); + assertNull(omMetadataManager.getVolumeTable().getSkipCache(volumeName)); } @Test From 91f4a1cc20b259a82ae7c5e492f6eecdaa3b6996 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Thu, 29 Oct 2026 03:40:31 +0800 Subject: [PATCH 2/2] HDDS-17240. Use the existing volume iterator and test helper --- .../request/volume/OMQuotaRepairRequest.java | 3 +-- .../volume/TestOMQuotaRepairRequest.java | 24 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMQuotaRepairRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMQuotaRepairRequest.java index bf75b321dc8c..25ad31881c3a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMQuotaRepairRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMQuotaRepairRequest.java @@ -174,8 +174,7 @@ private Map updateOldVolumeQuotaSupport( LOG.info("Starting volume quota support update"); Map volUpdateMap = new HashMap<>(); // The volume table is fully cached; iterate the cache to see volume changes not flushed yet. - Iterator, CacheValue>> iterator = - metadataManager.getVolumeTable().cacheIterator(); + Iterator, CacheValue>> iterator = metadataManager.getVolumeIterator(); while (iterator.hasNext()) { Map.Entry, CacheValue> entry = iterator.next(); String volumeKey = entry.getKey().getCacheKey(); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMQuotaRepairRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMQuotaRepairRequest.java index 39b53949a25c..de0301ea9547 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMQuotaRepairRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMQuotaRepairRequest.java @@ -163,15 +163,24 @@ public void testOldVolumeQuotaRepairKeepsUnflushedVolumeChanges() throws Excepti // Volume changes applied after those rows were flushed, still waiting for the next flush. Table volumeTable = omMetadataManager.getVolumeTable(); volumeTable.addCacheEntry(new CacheKey<>(deletedVolumeKey), CacheValue.get(2L)); - volumeTable.addCacheEntry(new CacheKey<>(usedVolumeKey), CacheValue.get(3L, - volumeTable.get(usedVolumeKey).toBuilder().setUsedNamespace(1).incRefCount().build())); - volumeTable.addCacheEntry(new CacheKey<>(quotaSetVolumeKey), CacheValue.get(4L, - volumeTable.get(quotaSetVolumeKey).toBuilder().setQuotaInNamespace(10).build())); + OmVolumeArgs unflushedUsedVolume = volumeTable.get(usedVolumeKey) + .toBuilder() + .setUsedNamespace(1) + .incRefCount() + .build(); + volumeTable.addCacheEntry(new CacheKey<>(usedVolumeKey), CacheValue.get(3L, unflushedUsedVolume)); + OmVolumeArgs unflushedQuotaSetVolume = volumeTable.get(quotaSetVolumeKey) + .toBuilder() + .setQuotaInNamespace(10) + .build(); + volumeTable.addCacheEntry(new CacheKey<>(quotaSetVolumeKey), CacheValue.get(4L, unflushedQuotaSetVolume)); OMRequest omRequest = OMRequest.newBuilder() .setClientId("test-client") .setCmdType(Type.QuotaRepair) - .setQuotaRepairRequest(QuotaRepairRequest.newBuilder().setSupportVolumeOldQuota(true).build()) + .setQuotaRepairRequest(QuotaRepairRequest.newBuilder() + .setSupportVolumeOldQuota(true) + .build()) .build(); OMClientResponse omClientResponse = new OMQuotaRepairRequest(omRequest).validateAndUpdateCache(ozoneManager, 5L); assertThat(omClientResponse.getOMResponse().getStatus()).isEqualTo(Status.OK); @@ -199,9 +208,10 @@ private String addOldQuotaVolume(boolean flushed) throws Exception { .build(); String volumeKey = omMetadataManager.getVolumeKey(volumeName); if (flushed) { - omMetadataManager.getVolumeTable().put(volumeKey, volumeArgs); + OMRequestTestUtils.addVolumeToOM(omMetadataManager, volumeArgs); + } else { + omMetadataManager.getVolumeTable().addCacheEntry(new CacheKey<>(volumeKey), CacheValue.get(1L, volumeArgs)); } - omMetadataManager.getVolumeTable().addCacheEntry(new CacheKey<>(volumeKey), CacheValue.get(1L, volumeArgs)); return volumeKey; } }