From a40d61a175d38f8bbfeff1dcf89c825336680e3e Mon Sep 17 00:00:00 2001 From: cxorm Date: Sat, 21 Dec 2019 18:06:59 +0800 Subject: [PATCH 01/16] Fix listing volumes with StartVolume --- .../hadoop/ozone/client/ObjectStore.java | 5 --- .../ozone/om/OmMetadataManagerImpl.java | 45 ++++++++++++++----- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index 46c8148015e1..b9e8fac23de6 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -299,11 +299,6 @@ private class VolumeIterator implements Iterator { @Override public boolean hasNext() { - if(!currentIterator.hasNext()) { - currentIterator = getNextListOfVolumes( - currentValue != null ? currentValue.getName() : null) - .iterator(); - } return currentIterator.hasNext(); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index 6d02a3df1056..7a84718e54c9 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -815,34 +815,55 @@ public List listVolumes(String userName, } volumes = getVolumesByUser(userName); - if (volumes == null || volumes.getVolumeNamesCount() == 0) { + if (volumes.getVolumeNamesCount() == 0) { return result; } + /* Process listing volumes with startVolume. */ boolean startKeyFound = Strings.isNullOrEmpty(startKey); + if (!startKeyFound) { + OmVolumeArgs startVolArgs = getVolumeTable() + .get(this.getVolumeKey(startKey)); + + if (startVolArgs == null) { + throw new OMException("StartVolume info not found for " + startKey, + ResultCodes.VOLUME_NOT_FOUND); + } else if (!startKey.startsWith(prefix)) { + throw new OMException("StartVolume " + startKey + " not found" + + " for prefix " + prefix, ResultCodes.VOLUME_NOT_FOUND); + } + + result.add(startVolArgs); + } + + OmVolumeArgs volumeArgs; for (String volumeName : volumes.getVolumeNamesList()) { + if (!Strings.isNullOrEmpty(prefix)) { if (!volumeName.startsWith(prefix)) { continue; } } + volumeArgs = getVolumeTable().get(this.getVolumeKey(volumeName)); + if (volumeArgs == null) { + // Could not get volume info by given volume name, + // since the volume name is loaded from db, + // this probably means om db is corrupted or some entries are + // accidentally removed. + throw new OMException("Volume info not found for " + volumeName, + ResultCodes.VOLUME_NOT_FOUND); + } + if (!startKeyFound && volumeName.equals(startKey)) { startKeyFound = true; continue; } - if (startKeyFound && result.size() < maxKeys) { - OmVolumeArgs volumeArgs = - getVolumeTable().get(this.getVolumeKey(volumeName)); - if (volumeArgs == null) { - // Could not get volume info by given volume name, - // since the volume name is loaded from db, - // this probably means om db is corrupted or some entries are - // accidentally removed. - throw new OMException("Volume info not found for " + volumeName, - ResultCodes.VOLUME_NOT_FOUND); - } + + if (result.size() < maxKeys) { result.add(volumeArgs); + } else { + break; } } From 03bcf0c3324fbfad4157b7b42756eefa640bfd8b Mon Sep 17 00:00:00 2001 From: cxorm Date: Sat, 21 Dec 2019 18:07:28 +0800 Subject: [PATCH 02/16] Update UTs --- .../ozone/om/TestOmMetadataManager.java | 31 +++++++++++++++++++ .../ozone/om/request/TestOMRequestUtils.java | 23 +++++++++----- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java index e0e4c61d3e54..182c37ee1aab 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java @@ -24,6 +24,7 @@ import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.junit.Assert; import org.junit.Before; @@ -55,6 +56,36 @@ public void setup() throws Exception { folder.getRoot().getAbsolutePath()); omMetadataManager = new OmMetadataManagerImpl(ozoneConfiguration); } + + @Test + public void testListVolumes() throws Exception { + String ownerName = "owner"; + OmVolumeArgs.Builder argsBuilder = OmVolumeArgs.newBuilder() + .setAdminName("admin") + .setOwnerName(ownerName); + + String volName; + char postfix = 'a'; + OmVolumeArgs omVolumeArgs; + for (int i = 0; i < 26; i++) { + volName = "vol" + (char)(postfix + i); + omVolumeArgs = argsBuilder + .setVolume(volName) + .build(); + + TestOMRequestUtils.addVolumeToOM(omMetadataManager, omVolumeArgs); + TestOMRequestUtils.addUserToDB(volName, ownerName, omMetadataManager); + } + + // Test list volumes with setting startVolume. + String prefix = ""; + String startVolume = "volz"; + List volumeList = omMetadataManager.listVolumes(ownerName, + prefix, startVolume, 100); + Assert.assertEquals(volumeList.get(0).getVolume(), startVolume); + Assert.assertEquals(volumeList.size(), 26); + } + @Test public void testListBuckets() throws Exception { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMRequestUtils.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMRequestUtils.java index 6ceced4077e5..49c204dcc665 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMRequestUtils.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMRequestUtils.java @@ -302,13 +302,22 @@ public static List< HddsProtos.KeyValue> getMetadataList() { */ public static void addUserToDB(String volumeName, String ownerName, OMMetadataManager omMetadataManager) throws Exception { - OzoneManagerProtocolProtos.UserVolumeInfo userVolumeInfo = - OzoneManagerProtocolProtos.UserVolumeInfo - .newBuilder() - .addVolumeNames(volumeName) - .setObjectID(1) - .setUpdateID(1) - .build(); + + OzoneManagerProtocolProtos.UserVolumeInfo userVolumeInfo = omMetadataManager + .getUserTable().get(omMetadataManager.getUserKey(ownerName)); + if (userVolumeInfo == null) { + userVolumeInfo = OzoneManagerProtocolProtos.UserVolumeInfo + .newBuilder() + .addVolumeNames(volumeName) + .setObjectID(1) + .setUpdateID(1) + .build(); + } else { + userVolumeInfo = userVolumeInfo.toBuilder() + .addVolumeNames(volumeName) + .build(); + } + omMetadataManager.getUserTable().put( omMetadataManager.getUserKey(ownerName), userVolumeInfo); } From df0aeae3f2ebb157a63b1c91223e10503c9cbc52 Mon Sep 17 00:00:00 2001 From: cxorm Date: Mon, 23 Dec 2019 15:39:17 +0800 Subject: [PATCH 03/16] Revert "Fix listing volumes with StartVolume" This reverts commit a40d61a175d38f8bbfeff1dcf89c825336680e3e. --- .../hadoop/ozone/client/ObjectStore.java | 5 +++ .../ozone/om/OmMetadataManagerImpl.java | 45 +++++-------------- 2 files changed, 17 insertions(+), 33 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index b9e8fac23de6..46c8148015e1 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -299,6 +299,11 @@ private class VolumeIterator implements Iterator { @Override public boolean hasNext() { + if(!currentIterator.hasNext()) { + currentIterator = getNextListOfVolumes( + currentValue != null ? currentValue.getName() : null) + .iterator(); + } return currentIterator.hasNext(); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index 7a84718e54c9..6d02a3df1056 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -815,55 +815,34 @@ public List listVolumes(String userName, } volumes = getVolumesByUser(userName); - if (volumes.getVolumeNamesCount() == 0) { + if (volumes == null || volumes.getVolumeNamesCount() == 0) { return result; } - /* Process listing volumes with startVolume. */ boolean startKeyFound = Strings.isNullOrEmpty(startKey); - if (!startKeyFound) { - OmVolumeArgs startVolArgs = getVolumeTable() - .get(this.getVolumeKey(startKey)); - - if (startVolArgs == null) { - throw new OMException("StartVolume info not found for " + startKey, - ResultCodes.VOLUME_NOT_FOUND); - } else if (!startKey.startsWith(prefix)) { - throw new OMException("StartVolume " + startKey + " not found" + - " for prefix " + prefix, ResultCodes.VOLUME_NOT_FOUND); - } - - result.add(startVolArgs); - } - - OmVolumeArgs volumeArgs; for (String volumeName : volumes.getVolumeNamesList()) { - if (!Strings.isNullOrEmpty(prefix)) { if (!volumeName.startsWith(prefix)) { continue; } } - volumeArgs = getVolumeTable().get(this.getVolumeKey(volumeName)); - if (volumeArgs == null) { - // Could not get volume info by given volume name, - // since the volume name is loaded from db, - // this probably means om db is corrupted or some entries are - // accidentally removed. - throw new OMException("Volume info not found for " + volumeName, - ResultCodes.VOLUME_NOT_FOUND); - } - if (!startKeyFound && volumeName.equals(startKey)) { startKeyFound = true; continue; } - - if (result.size() < maxKeys) { + if (startKeyFound && result.size() < maxKeys) { + OmVolumeArgs volumeArgs = + getVolumeTable().get(this.getVolumeKey(volumeName)); + if (volumeArgs == null) { + // Could not get volume info by given volume name, + // since the volume name is loaded from db, + // this probably means om db is corrupted or some entries are + // accidentally removed. + throw new OMException("Volume info not found for " + volumeName, + ResultCodes.VOLUME_NOT_FOUND); + } result.add(volumeArgs); - } else { - break; } } From 39dec1a77f57e7a8a6645ca61e434dfab2895006 Mon Sep 17 00:00:00 2001 From: cxorm Date: Mon, 23 Dec 2019 16:39:41 +0800 Subject: [PATCH 04/16] Fix listing volumes with StartVolume --- .../ozone/om/OmMetadataManagerImpl.java | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index 6d02a3df1056..d49024c6c13f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -807,33 +807,30 @@ public List listTrash(String volumeName, String bucketName, @Override public List listVolumes(String userName, String prefix, String startKey, int maxKeys) throws IOException { - List result = Lists.newArrayList(); - UserVolumeInfo volumes; + if (StringUtil.isBlank(userName)) { throw new OMException("User name is required to list Volumes.", ResultCodes.USER_NOT_FOUND); } - volumes = getVolumesByUser(userName); - if (volumes == null || volumes.getVolumeNamesCount() == 0) { - return result; - } + final List result = Lists.newArrayList(); + final List volumes = getVolumesByUser(userName) + .getVolumeNamesList(); - boolean startKeyFound = Strings.isNullOrEmpty(startKey); - for (String volumeName : volumes.getVolumeNamesList()) { - if (!Strings.isNullOrEmpty(prefix)) { - if (!volumeName.startsWith(prefix)) { - continue; - } - } + int index = 0; + if (!Strings.isNullOrEmpty(startKey)) { + index = volumes.indexOf( + startKey.startsWith(OzoneConsts.OM_KEY_PREFIX) ? + startKey.substring(1) : + startKey); + } + final String startChar = prefix == null ? "" : prefix; - if (!startKeyFound && volumeName.equals(startKey)) { - startKeyFound = true; - continue; - } - if (startKeyFound && result.size() < maxKeys) { - OmVolumeArgs volumeArgs = - getVolumeTable().get(this.getVolumeKey(volumeName)); + while (index != -1 && index < volumes.size() && result.size() < maxKeys) { + final String volumeName = volumes.get(index); + if (volumeName.startsWith(startChar)) { + final OmVolumeArgs volumeArgs = getVolumeTable() + .get(getVolumeKey(volumeName)); if (volumeArgs == null) { // Could not get volume info by given volume name, // since the volume name is loaded from db, @@ -844,6 +841,7 @@ public List listVolumes(String userName, } result.add(volumeArgs); } + index++; } return result; From 4e7a7f34959835ec9321a75a605f4d46bd8a0491 Mon Sep 17 00:00:00 2001 From: cxorm Date: Mon, 23 Dec 2019 16:40:12 +0800 Subject: [PATCH 05/16] Fix UT --- .../apache/hadoop/ozone/om/TestOmMetadataManager.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java index 182c37ee1aab..9cf3261112cd 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java @@ -65,10 +65,9 @@ public void testListVolumes() throws Exception { .setOwnerName(ownerName); String volName; - char postfix = 'a'; OmVolumeArgs omVolumeArgs; - for (int i = 0; i < 26; i++) { - volName = "vol" + (char)(postfix + i); + for (int i = 0; i < 50; i++) { + volName = "vol" + i; omVolumeArgs = argsBuilder .setVolume(volName) .build(); @@ -79,11 +78,11 @@ public void testListVolumes() throws Exception { // Test list volumes with setting startVolume. String prefix = ""; - String startVolume = "volz"; + String startVolume = "vol25"; List volumeList = omMetadataManager.listVolumes(ownerName, prefix, startVolume, 100); Assert.assertEquals(volumeList.get(0).getVolume(), startVolume); - Assert.assertEquals(volumeList.size(), 26); + Assert.assertEquals(volumeList.size(), 25); } @Test From 1925ffbc7c6d2cf8d3ee68fee5dd84e20a75937d Mon Sep 17 00:00:00 2001 From: cxorm Date: Mon, 23 Dec 2019 20:03:45 +0800 Subject: [PATCH 06/16] Fix VolumeIterator#hasNext --- .../org/apache/hadoop/ozone/client/ObjectStore.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index 46c8148015e1..e983007d86f1 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -282,6 +282,7 @@ private class VolumeIterator implements Iterator { private Iterator currentIterator; private OzoneVolume currentValue; + private OzoneVolume repeatedVolume; /** * Creates an Iterator to iterate over all volumes after @@ -294,6 +295,7 @@ private class VolumeIterator implements Iterator { this.user = user; this.volPrefix = volPrefix; this.currentValue = null; + this.repeatedVolume = getNextListOfVolumes(prevVolume).iterator().next(); this.currentIterator = getNextListOfVolumes(prevVolume).iterator(); } @@ -303,7 +305,16 @@ public boolean hasNext() { currentIterator = getNextListOfVolumes( currentValue != null ? currentValue.getName() : null) .iterator(); + + /* Cause getNextListOfVolumes never return empty-list, + * we should check whether the volume is fetched or not. + * */ + if (currentIterator.hasNext() && + currentIterator.next().equals(repeatedVolume)) { + return false; + } } + return currentIterator.hasNext(); } From 4c1adfcaea5fe11681d2a7e93da7804a11a44f59 Mon Sep 17 00:00:00 2001 From: cxorm Date: Tue, 24 Dec 2019 17:21:32 +0800 Subject: [PATCH 07/16] Revert "Fix VolumeIterator#hasNext" This reverts commit 1925ffbc7c6d2cf8d3ee68fee5dd84e20a75937d. --- .../org/apache/hadoop/ozone/client/ObjectStore.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index e983007d86f1..46c8148015e1 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -282,7 +282,6 @@ private class VolumeIterator implements Iterator { private Iterator currentIterator; private OzoneVolume currentValue; - private OzoneVolume repeatedVolume; /** * Creates an Iterator to iterate over all volumes after @@ -295,7 +294,6 @@ private class VolumeIterator implements Iterator { this.user = user; this.volPrefix = volPrefix; this.currentValue = null; - this.repeatedVolume = getNextListOfVolumes(prevVolume).iterator().next(); this.currentIterator = getNextListOfVolumes(prevVolume).iterator(); } @@ -305,16 +303,7 @@ public boolean hasNext() { currentIterator = getNextListOfVolumes( currentValue != null ? currentValue.getName() : null) .iterator(); - - /* Cause getNextListOfVolumes never return empty-list, - * we should check whether the volume is fetched or not. - * */ - if (currentIterator.hasNext() && - currentIterator.next().equals(repeatedVolume)) { - return false; - } } - return currentIterator.hasNext(); } From 7eda1a09ae6e5790668ec205a15934743a956328 Mon Sep 17 00:00:00 2001 From: cxorm Date: Tue, 24 Dec 2019 17:48:46 +0800 Subject: [PATCH 08/16] Fix VolumeIterator --- .../hadoop/ozone/client/ObjectStore.java | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index 46c8148015e1..d529ee6ff5b9 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -290,11 +290,11 @@ private class VolumeIterator implements Iterator { * @param user user name * @param volPrefix volume prefix to match */ - VolumeIterator(String user, String volPrefix, String prevVolume) { + VolumeIterator(String user, String volPrefix, String initVolume) { this.user = user; this.volPrefix = volPrefix; this.currentValue = null; - this.currentIterator = getNextListOfVolumes(prevVolume).iterator(); + this.currentIterator = getInitListOfVolumes(initVolume).iterator(); } @Override @@ -316,21 +316,44 @@ public OzoneVolume next() { throw new NoSuchElementException(); } + /** + * Returns the initial set of volume list using proxy. + * @return {@code List} + */ + private List getInitListOfVolumes(String initVolume) { + try { + //if user is null, we do list of all volumes. + if(user != null) { + return proxy.listVolumes(user, volPrefix, initVolume, listCacheSize); + } + return proxy.listVolumes(volPrefix, initVolume, listCacheSize); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + /** * Returns the next set of volume list using proxy. * @param prevVolume previous volume, this will be excluded from the result * @return {@code List} */ private List getNextListOfVolumes(String prevVolume) { + List volumeList; try { //if user is null, we do list of all volumes. if(user != null) { - return proxy.listVolumes(user, volPrefix, prevVolume, listCacheSize); + volumeList = proxy + .listVolumes(user, volPrefix, prevVolume, listCacheSize); + } else { + volumeList = proxy + .listVolumes(volPrefix, prevVolume, listCacheSize); } - return proxy.listVolumes(volPrefix, prevVolume, listCacheSize); } catch (IOException e) { throw new RuntimeException(e); } + + volumeList.remove(0); + return volumeList; } } From 8691191b2e7fdbd90b54ae7d1fbf19de34f82e07 Mon Sep 17 00:00:00 2001 From: cxorm Date: Wed, 25 Dec 2019 01:09:51 +0800 Subject: [PATCH 09/16] Update VolumeIterator#hasNext to handle startVol not found --- .../java/org/apache/hadoop/ozone/client/ObjectStore.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index d529ee6ff5b9..1bc124866b00 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -300,9 +300,12 @@ private class VolumeIterator implements Iterator { @Override public boolean hasNext() { if(!currentIterator.hasNext()) { - currentIterator = getNextListOfVolumes( - currentValue != null ? currentValue.getName() : null) - .iterator(); + if (currentValue != null) { + currentIterator = getNextListOfVolumes(currentValue.getName()) + .iterator(); + } else { + return false; + } } return currentIterator.hasNext(); } From f7719ab628e331669e8dfa6a082548fc5634fb82 Mon Sep 17 00:00:00 2001 From: cxorm Date: Sat, 4 Jan 2020 16:20:48 +0800 Subject: [PATCH 10/16] Revert "Fix VolumeIterator" This reverts commit 7eda1a09ae6e5790668ec205a15934743a956328. --- .../hadoop/ozone/client/ObjectStore.java | 31 +++---------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index 1bc124866b00..43714fc725b3 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -290,11 +290,11 @@ private class VolumeIterator implements Iterator { * @param user user name * @param volPrefix volume prefix to match */ - VolumeIterator(String user, String volPrefix, String initVolume) { + VolumeIterator(String user, String volPrefix, String prevVolume) { this.user = user; this.volPrefix = volPrefix; this.currentValue = null; - this.currentIterator = getInitListOfVolumes(initVolume).iterator(); + this.currentIterator = getNextListOfVolumes(prevVolume).iterator(); } @Override @@ -319,44 +319,21 @@ public OzoneVolume next() { throw new NoSuchElementException(); } - /** - * Returns the initial set of volume list using proxy. - * @return {@code List} - */ - private List getInitListOfVolumes(String initVolume) { - try { - //if user is null, we do list of all volumes. - if(user != null) { - return proxy.listVolumes(user, volPrefix, initVolume, listCacheSize); - } - return proxy.listVolumes(volPrefix, initVolume, listCacheSize); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - /** * Returns the next set of volume list using proxy. * @param prevVolume previous volume, this will be excluded from the result * @return {@code List} */ private List getNextListOfVolumes(String prevVolume) { - List volumeList; try { //if user is null, we do list of all volumes. if(user != null) { - volumeList = proxy - .listVolumes(user, volPrefix, prevVolume, listCacheSize); - } else { - volumeList = proxy - .listVolumes(volPrefix, prevVolume, listCacheSize); + return proxy.listVolumes(user, volPrefix, prevVolume, listCacheSize); } + return proxy.listVolumes(volPrefix, prevVolume, listCacheSize); } catch (IOException e) { throw new RuntimeException(e); } - - volumeList.remove(0); - return volumeList; } } From bde39769cc2c4ba65ae313ab5668ef45d5622a0f Mon Sep 17 00:00:00 2001 From: cxorm Date: Sat, 4 Jan 2020 16:21:30 +0800 Subject: [PATCH 11/16] Revert "Update VolumeIterator#hasNext to handle startVol not found" This reverts commit 8691191b2e7fdbd90b54ae7d1fbf19de34f82e07. --- .../java/org/apache/hadoop/ozone/client/ObjectStore.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index 43714fc725b3..46c8148015e1 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -300,12 +300,9 @@ private class VolumeIterator implements Iterator { @Override public boolean hasNext() { if(!currentIterator.hasNext()) { - if (currentValue != null) { - currentIterator = getNextListOfVolumes(currentValue.getName()) - .iterator(); - } else { - return false; - } + currentIterator = getNextListOfVolumes( + currentValue != null ? currentValue.getName() : null) + .iterator(); } return currentIterator.hasNext(); } From f7be6cbb137f8eb6ef5979f843f77b32828a4021 Mon Sep 17 00:00:00 2001 From: cxorm Date: Sat, 4 Jan 2020 17:52:45 +0800 Subject: [PATCH 12/16] Exclude startVolume as part of list --- .../java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index d49024c6c13f..bdc31e5f155e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -823,6 +823,9 @@ public List listVolumes(String userName, startKey.startsWith(OzoneConsts.OM_KEY_PREFIX) ? startKey.substring(1) : startKey); + + // Exclude the startVolume as part of the result. + index = index != -1 ? index + 1 : index; } final String startChar = prefix == null ? "" : prefix; From 1958f4599cb719ce78cb894b5357bf54af0c456a Mon Sep 17 00:00:00 2001 From: cxorm Date: Sat, 4 Jan 2020 17:53:12 +0800 Subject: [PATCH 13/16] Update UT --- .../hadoop/ozone/om/TestOmMetadataManager.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java index 9cf3261112cd..6802afcd620e 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java @@ -76,13 +76,17 @@ public void testListVolumes() throws Exception { TestOMRequestUtils.addUserToDB(volName, ownerName, omMetadataManager); } - // Test list volumes with setting startVolume. + // Test list volumes with setting startVolume that + // was not part of the result. String prefix = ""; - String startVolume = "vol25"; + int totalVol = omMetadataManager + .listVolumes(ownerName, prefix, null, 100) + .size(); + int startOrder = 10; + String startVolume = "vol" + startOrder; List volumeList = omMetadataManager.listVolumes(ownerName, prefix, startVolume, 100); - Assert.assertEquals(volumeList.get(0).getVolume(), startVolume); - Assert.assertEquals(volumeList.size(), 25); + Assert.assertEquals(volumeList.size(), totalVol - startOrder - 1); } @Test From 33c52b3f7ee8c910802674b2e350fe62a73f7c35 Mon Sep 17 00:00:00 2001 From: cxorm Date: Sun, 5 Jan 2020 14:00:35 +0800 Subject: [PATCH 14/16] Update description of startVolume --- .../hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java index de78b4fc76de..f2b645952884 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java @@ -51,7 +51,7 @@ public class ListVolumeHandler extends Handler { private int maxVolumes; @Option(names = {"--start", "-s"}, - description = "The first volume to start the listing") + description = "The listing will start from volume after the startVolume") private String startVolume; @Option(names = {"--prefix", "-p"}, From d5dac4e37f1b09c06a4c7bd4585daffa529484b3 Mon Sep 17 00:00:00 2001 From: cxorm Date: Mon, 6 Jan 2020 20:48:21 +0800 Subject: [PATCH 15/16] Update description of startVolume --- .../hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java index f2b645952884..6a09f12bb338 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/ListVolumeHandler.java @@ -51,7 +51,8 @@ public class ListVolumeHandler extends Handler { private int maxVolumes; @Option(names = {"--start", "-s"}, - description = "The listing will start from volume after the startVolume") + description = "The volume to start the listing from.\n" + + "This will be excluded from the result.") private String startVolume; @Option(names = {"--prefix", "-p"}, From 76bd8c2de1e5b72e753e545b679d71459258dc8b Mon Sep 17 00:00:00 2001 From: cxorm Date: Mon, 6 Jan 2020 21:11:06 +0800 Subject: [PATCH 16/16] List empty result when setting --start with last volume --- .../java/org/apache/hadoop/ozone/client/ObjectStore.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java index 46c8148015e1..253aa0402f23 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java @@ -299,9 +299,8 @@ private class VolumeIterator implements Iterator { @Override public boolean hasNext() { - if(!currentIterator.hasNext()) { - currentIterator = getNextListOfVolumes( - currentValue != null ? currentValue.getName() : null) + if (!currentIterator.hasNext() && currentValue != null) { + currentIterator = getNextListOfVolumes(currentValue.getName()) .iterator(); } return currentIterator.hasNext();