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..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 @@ -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,43 @@ 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.getVolumeIterator(); + 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..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 @@ -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,67 @@ 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)); + 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()) + .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) { + OMRequestTestUtils.addVolumeToOM(omMetadataManager, volumeArgs); + } else { + 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