From fb1ce0d50a4a2b2feea6833b616b180084e0801c Mon Sep 17 00:00:00 2001 From: Vivek Ratnavel Subramanian Date: Tue, 18 Jun 2019 08:51:16 -0700 Subject: [PATCH] HDDS-1670. Add limit support to /api/containers and /api/containers/{id} endpoints (#954) --- .../ozone/recon/api/ContainerKeyService.java | 19 +++++-- .../recon/spi/ContainerDBServiceProvider.java | 12 ++++- .../impl/ContainerDBServiceProviderImpl.java | 27 ++++++++-- .../recon/api/TestContainerKeyService.java | 51 +++++++++++-------- 4 files changed, 80 insertions(+), 29 deletions(-) diff --git a/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerKeyService.java b/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerKeyService.java index 35ae724b93f..ff355dbac27 100644 --- a/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerKeyService.java +++ b/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerKeyService.java @@ -21,17 +21,19 @@ import java.io.IOException; import java.time.Instant; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.TreeMap; import java.util.stream.Collectors; import javax.inject.Inject; +import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -72,10 +74,11 @@ public class ContainerKeyService { * @return {@link Response} */ @GET - public Response getContainers() { + public Response getContainers( + @DefaultValue("-1") @QueryParam("limit") int limit) { Map containersMap; try { - containersMap = containerDBServiceProvider.getContainers(); + containersMap = containerDBServiceProvider.getContainers(limit); } catch (IOException ioEx) { throw new WebApplicationException(ioEx, Response.Status.INTERNAL_SERVER_ERROR); @@ -92,8 +95,10 @@ public class ContainerKeyService { */ @GET @Path("/{id}") - public Response getKeysForContainer(@PathParam("id") Long containerId) { - Map keyMetadataMap = new HashMap<>(); + public Response getKeysForContainer( + @PathParam("id") Long containerId, + @DefaultValue("-1") @QueryParam("limit") int limit) { + Map keyMetadataMap = new LinkedHashMap<>(); try { Map containerKeyPrefixMap = containerDBServiceProvider.getKeyPrefixesForContainer(containerId); @@ -143,6 +148,10 @@ public class ContainerKeyService { Collections.singletonMap(containerKeyPrefix.getKeyVersion(), blockIds)); } else { + // break the for loop if limit has been reached + if (keyMetadataMap.size() == limit) { + break; + } KeyMetadata keyMetadata = new KeyMetadata(); keyMetadata.setBucket(omKeyInfo.getBucketName()); keyMetadata.setVolume(omKeyInfo.getVolumeName()); diff --git a/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ContainerDBServiceProvider.java b/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ContainerDBServiceProvider.java index 0449e7cf774..93178b0fde0 100644 --- a/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ContainerDBServiceProvider.java +++ b/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ContainerDBServiceProvider.java @@ -70,13 +70,23 @@ public interface ContainerDBServiceProvider { throws IOException; /** - * Get a Map of containerID, containerMetadata of all Containers. + * Get a Map of containerID, containerMetadata of all the Containers. * * @return Map of containerID -> containerMetadata. * @throws IOException */ Map getContainers() throws IOException; + /** + * Get a Map of containerID, containerMetadata of Containers only for the + * given limit. If the limit is -1 or any integer <0, then return all + * the containers without any limit. + * + * @return Map of containerID -> containerMetadata. + * @throws IOException + */ + Map getContainers(int limit) throws IOException; + /** * Delete an entry in the container DB. * @param containerKeyPrefix container key prefix to be deleted. diff --git a/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ContainerDBServiceProviderImpl.java b/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ContainerDBServiceProviderImpl.java index e79b8044f08..532f74792c0 100644 --- a/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ContainerDBServiceProviderImpl.java +++ b/hadoop-ozone/ozone-recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ContainerDBServiceProviderImpl.java @@ -22,7 +22,6 @@ import static org.apache.hadoop.ozone.recon.ReconConstants.CONTAINER_KEY_TABLE; import java.io.File; import java.io.IOException; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; @@ -139,7 +138,7 @@ public class ContainerDBServiceProviderImpl public Map getKeyPrefixesForContainer( long containerId) throws IOException { - Map prefixes = new HashMap<>(); + Map prefixes = new LinkedHashMap<>(); TableIterator> containerIterator = containerKeyTable.iterator(); containerIterator.seek(new ContainerKeyPrefix(containerId)); @@ -166,13 +165,29 @@ public class ContainerDBServiceProviderImpl } /** - * Iterate the DB to construct a Map of containerID -> containerMetadata. + * Get all the containers. * * @return Map of containerID -> containerMetadata. * @throws IOException */ @Override public Map getContainers() throws IOException { + // Set a negative limit to get all the containers. + return getContainers(-1); + } + + /** + * Iterate the DB to construct a Map of containerID -> containerMetadata + * only for the given limit. + * + * Return all the containers if limit < 0. + * + * @return Map of containerID -> containerMetadata. + * @throws IOException + */ + @Override + public Map getContainers(int limit) + throws IOException { Map containers = new LinkedHashMap<>(); TableIterator> containerIterator = containerKeyTable.iterator(); @@ -181,6 +196,12 @@ public class ContainerDBServiceProviderImpl Long containerID = keyValue.getKey().getContainerId(); Integer numberOfKeys = keyValue.getValue(); + // break the loop if limit has been reached + // and one more new entity needs to be added to the containers map + if (containers.size() == limit && !containers.containsKey(containerID)) { + break; + } + // initialize containerMetadata with 0 as number of keys. containers.computeIfAbsent(containerID, ContainerMetadata::new); // increment number of keys for the containerID diff --git a/hadoop-ozone/ozone-recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerKeyService.java b/hadoop-ozone/ozone-recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerKeyService.java index 6363e9c4bd6..620e03b9279 100644 --- a/hadoop-ozone/ozone-recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerKeyService.java +++ b/hadoop-ozone/ozone-recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerKeyService.java @@ -20,6 +20,7 @@ package org.apache.hadoop.ozone.recon.api; import static org.apache.hadoop.ozone.recon.ReconServerConfigKeys.OZONE_RECON_DB_DIR; import static org.apache.hadoop.ozone.recon.ReconServerConfigKeys.OZONE_RECON_OM_SNAPSHOT_DB_DIR; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import java.io.File; @@ -200,56 +201,67 @@ public class TestContainerKeyService extends AbstractOMMetadataManagerTest { @Test public void testGetKeysForContainer() { - Response response = containerKeyService.getKeysForContainer(1L); + Response response = containerKeyService.getKeysForContainer(1L, -1); Collection keyMetadataList = (Collection) response.getEntity(); - assertTrue(keyMetadataList.size() == 2); + assertEquals(keyMetadataList.size(), 2); Iterator iterator = keyMetadataList.iterator(); KeyMetadata keyMetadata = iterator.next(); - assertTrue(keyMetadata.getKey().equals("key_one")); - assertTrue(keyMetadata.getVersions().size() == 1); - assertTrue(keyMetadata.getBlockIds().size() == 1); + assertEquals(keyMetadata.getKey(), "key_one"); + assertEquals(keyMetadata.getVersions().size(), 1); + assertEquals(keyMetadata.getBlockIds().size(), 1); Map> blockIds = keyMetadata.getBlockIds(); - assertTrue(blockIds.get(0L).iterator().next().getLocalID() == 101); + assertEquals(blockIds.get(0L).iterator().next().getLocalID(), 101); keyMetadata = iterator.next(); - assertTrue(keyMetadata.getKey().equals("key_two")); - assertTrue(keyMetadata.getVersions().size() == 2); + assertEquals(keyMetadata.getKey(), "key_two"); + assertEquals(keyMetadata.getVersions().size(), 2); assertTrue(keyMetadata.getVersions().contains(0L) && keyMetadata .getVersions().contains(1L)); - assertTrue(keyMetadata.getBlockIds().size() == 2); + assertEquals(keyMetadata.getBlockIds().size(), 2); blockIds = keyMetadata.getBlockIds(); - assertTrue(blockIds.get(0L).iterator().next().getLocalID() == 103); - assertTrue(blockIds.get(1L).iterator().next().getLocalID() == 104); + assertEquals(blockIds.get(0L).iterator().next().getLocalID(), 103); + assertEquals(blockIds.get(1L).iterator().next().getLocalID(), 104); - response = containerKeyService.getKeysForContainer(3L); + response = containerKeyService.getKeysForContainer(3L, -1); keyMetadataList = (Collection) response.getEntity(); assertTrue(keyMetadataList.isEmpty()); + + // test if limit works as expected + response = containerKeyService.getKeysForContainer(1L, 1); + keyMetadataList = (Collection) response.getEntity(); + assertEquals(keyMetadataList.size(), 1); } @Test public void testGetContainers() { - Response response = containerKeyService.getContainers(); + Response response = containerKeyService.getContainers(-1); List containers = new ArrayList<>( (Collection) response.getEntity()); - assertTrue(containers.size() == 2); - Iterator iterator = containers.iterator(); ContainerMetadata containerMetadata = iterator.next(); - assertTrue(containerMetadata.getContainerID() == 1L); - assertTrue(containerMetadata.getNumberOfKeys() == 3L); + assertEquals(containerMetadata.getContainerID(), 1L); + // Number of keys for CID:1 should be 3 because of two different versions + // of key_two stored in CID:1 + assertEquals(containerMetadata.getNumberOfKeys(), 3L); containerMetadata = iterator.next(); - assertTrue(containerMetadata.getContainerID() == 2L); - assertTrue(containerMetadata.getNumberOfKeys() == 2L); + assertEquals(containerMetadata.getContainerID(), 2L); + assertEquals(containerMetadata.getNumberOfKeys(), 2L); + + // test if limit works as expected + response = containerKeyService.getContainers(1); + containers = new ArrayList<>( + (Collection) response.getEntity()); + assertEquals(containers.size(), 1); } /** @@ -266,5 +278,4 @@ public class TestContainerKeyService extends AbstractOMMetadataManagerTest { .getAbsolutePath()); return configuration; } - } \ No newline at end of file