You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by hi...@apache.org on 2019/06/16 00:55:58 UTC

[incubator-druid] branch master updated: endpoint to delete lookup tier and remove tier on last lookup deletion (#7852)

This is an automated email from the ASF dual-hosted git repository.

himanshug pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git


The following commit(s) were added to refs/heads/master by this push:
     new b3328b2  endpoint to delete lookup tier and remove tier on last lookup deletion (#7852)
b3328b2 is described below

commit b3328b2785884a4677338d97a116ef1fdb54459f
Author: Himanshu <g....@gmail.com>
AuthorDate: Sat Jun 15 17:55:50 2019 -0700

    endpoint to delete lookup tier and remove tier on last lookup deletion (#7852)
---
 docs/content/querying/lookups.md                   |  5 +-
 .../server/http/LookupCoordinatorResource.java     | 29 ++++++++
 .../lookup/cache/LookupCoordinatorManager.java     | 28 ++++++-
 .../server/http/LookupCoordinatorResourceTest.java | 42 +++++++++++
 .../lookup/cache/LookupCoordinatorManagerTest.java | 87 ++++++++++++++++++++++
 5 files changed, 189 insertions(+), 2 deletions(-)

diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md
index b54f769..7af2bcd 100644
--- a/docs/content/querying/lookups.md
+++ b/docs/content/querying/lookups.md
@@ -292,7 +292,10 @@ Using the prior example, a `GET` to `/druid/coordinator/v1/lookups/config/realti
 ```
 
 ## Delete Lookup
-A `DELETE` to `/druid/coordinator/v1/lookups/config/{tier}/{id}` will remove that lookup from the cluster.
+A `DELETE` to `/druid/coordinator/v1/lookups/config/{tier}/{id}` will remove that lookup from the cluster. If it was last lookup in the tier, then tier is deleted as well.
+
+## Delete Tier
+A `DELETE` to `/druid/coordinator/v1/lookups/config/{tier}` will remove that tier from the cluster.
 
 ## List tier names
 A `GET` to `/druid/coordinator/v1/lookups/config` will return a list of known tier names in the dynamic configuration.
diff --git a/server/src/main/java/org/apache/druid/server/http/LookupCoordinatorResource.java b/server/src/main/java/org/apache/druid/server/http/LookupCoordinatorResource.java
index 7affefe..f575211 100644
--- a/server/src/main/java/org/apache/druid/server/http/LookupCoordinatorResource.java
+++ b/server/src/main/java/org/apache/druid/server/http/LookupCoordinatorResource.java
@@ -178,6 +178,35 @@ public class LookupCoordinatorResource
 
   @DELETE
   @Produces({MediaType.APPLICATION_JSON, SmileMediaTypes.APPLICATION_JACKSON_SMILE})
+  @Path("/config/{tier}")
+  public Response deleteTier(
+      @PathParam("tier") String tier,
+      @HeaderParam(AuditManager.X_DRUID_AUTHOR) @DefaultValue("") final String author,
+      @HeaderParam(AuditManager.X_DRUID_COMMENT) @DefaultValue("") final String comment,
+      @Context HttpServletRequest req
+  )
+  {
+    try {
+      if (Strings.isNullOrEmpty(tier)) {
+        return Response.status(Response.Status.BAD_REQUEST)
+                       .entity(ServletResourceUtils.sanitizeException(new NullPointerException("`tier` required")))
+                       .build();
+      }
+
+      if (lookupCoordinatorManager.deleteTier(tier, new AuditInfo(author, comment, req.getRemoteAddr()))) {
+        return Response.status(Response.Status.ACCEPTED).build();
+      } else {
+        return Response.status(Response.Status.NOT_FOUND).build();
+      }
+    }
+    catch (Exception e) {
+      LOG.error(e, "Error deleting tier [%s]", tier);
+      return Response.serverError().entity(ServletResourceUtils.sanitizeException(e)).build();
+    }
+  }
+
+  @DELETE
+  @Produces({MediaType.APPLICATION_JSON, SmileMediaTypes.APPLICATION_JACKSON_SMILE})
   @Path("/config/{tier}/{lookup}")
   public Response deleteLookup(
       @PathParam("tier") String tier,
diff --git a/server/src/main/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManager.java b/server/src/main/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManager.java
index 86acfa3..b6589aa 100644
--- a/server/src/main/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManager.java
+++ b/server/src/main/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManager.java
@@ -255,6 +255,27 @@ public class LookupCoordinatorManager
     return lookupMapConfigRef.get();
   }
 
+  public boolean deleteTier(final String tier, AuditInfo auditInfo)
+  {
+    Preconditions.checkState(lifecycleLock.awaitStarted(5, TimeUnit.SECONDS), "not started");
+
+    synchronized (this) {
+      final Map<String, Map<String, LookupExtractorFactoryMapContainer>> priorSpec = getKnownLookups();
+      if (priorSpec == null) {
+        LOG.warn("Requested delete tier [%s]. But no lookups exist!", tier);
+        return false;
+      }
+      final Map<String, Map<String, LookupExtractorFactoryMapContainer>> updateSpec = new HashMap<>(priorSpec);
+
+      if (updateSpec.remove(tier) == null) {
+        LOG.warn("Requested delete of tier [%s] that does not exist!", tier);
+        return false;
+      }
+
+      return configManager.set(LOOKUP_CONFIG_KEY, updateSpec, auditInfo).isOk();
+    }
+  }
+
   public boolean deleteLookup(final String tier, final String lookup, AuditInfo auditInfo)
   {
     Preconditions.checkState(lifecycleLock.awaitStarted(5, TimeUnit.SECONDS), "not started");
@@ -279,7 +300,12 @@ public class LookupCoordinatorManager
 
       final Map<String, LookupExtractorFactoryMapContainer> updateTierSpec = new HashMap<>(priorTierSpec);
       updateTierSpec.remove(lookup);
-      updateSpec.put(tier, updateTierSpec);
+
+      if (updateTierSpec.isEmpty()) {
+        updateSpec.remove(tier);
+      } else {
+        updateSpec.put(tier, updateTierSpec);
+      }
       return configManager.set(LOOKUP_CONFIG_KEY, updateSpec, auditInfo).isOk();
     }
   }
diff --git a/server/src/test/java/org/apache/druid/server/http/LookupCoordinatorResourceTest.java b/server/src/test/java/org/apache/druid/server/http/LookupCoordinatorResourceTest.java
index fb6e22d..d1e673d 100644
--- a/server/src/test/java/org/apache/druid/server/http/LookupCoordinatorResourceTest.java
+++ b/server/src/test/java/org/apache/druid/server/http/LookupCoordinatorResourceTest.java
@@ -287,6 +287,48 @@ public class LookupCoordinatorResourceTest
   }
 
   @Test
+  public void testSimpleDeleteTier()
+  {
+    final String author = "some author";
+    final String comment = "some comment";
+    final String ip = "127.0.0.1";
+
+    final HttpServletRequest request = EasyMock.createStrictMock(HttpServletRequest.class);
+    EasyMock.expect(request.getRemoteAddr()).andReturn(ip).once();
+
+    final Capture<AuditInfo> auditInfoCapture = Capture.newInstance();
+    final LookupCoordinatorManager lookupCoordinatorManager = EasyMock.createStrictMock(
+        LookupCoordinatorManager.class);
+    EasyMock.expect(lookupCoordinatorManager.deleteTier(
+        EasyMock.eq(LOOKUP_TIER),
+        EasyMock.capture(auditInfoCapture)
+    )).andReturn(true).once();
+
+    EasyMock.replay(lookupCoordinatorManager, request);
+
+    final LookupCoordinatorResource lookupCoordinatorResource = new LookupCoordinatorResource(
+        lookupCoordinatorManager,
+        mapper,
+        mapper
+    );
+    final Response response = lookupCoordinatorResource.deleteTier(
+        LOOKUP_TIER,
+        author,
+        comment,
+        request
+    );
+
+    Assert.assertEquals(202, response.getStatus());
+    Assert.assertTrue(auditInfoCapture.hasCaptured());
+    final AuditInfo auditInfo = auditInfoCapture.getValue();
+    Assert.assertEquals(author, auditInfo.getAuthor());
+    Assert.assertEquals(comment, auditInfo.getComment());
+    Assert.assertEquals(ip, auditInfo.getIp());
+
+    EasyMock.verify(lookupCoordinatorManager, request);
+  }
+
+  @Test
   public void testSimpleDelete()
   {
     final String author = "some author";
diff --git a/server/src/test/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManagerTest.java b/server/src/test/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManagerTest.java
index aa32b81..772b185 100644
--- a/server/src/test/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManagerTest.java
+++ b/server/src/test/java/org/apache/druid/server/lookup/cache/LookupCoordinatorManagerTest.java
@@ -847,6 +847,52 @@ public class LookupCoordinatorManagerTest
   }
 
   @Test
+  public void testDeleteTier()
+  {
+    final LookupExtractorFactoryMapContainer foo1 = new LookupExtractorFactoryMapContainer(
+        "v0",
+        ImmutableMap.of("lookup", "foo1")
+    );
+
+    final LookupExtractorFactoryMapContainer foo2 = new LookupExtractorFactoryMapContainer(
+        "v0",
+        ImmutableMap.of("lookup", "foo2")
+    );
+    final LookupCoordinatorManager manager = new LookupCoordinatorManager(
+        client,
+        druidNodeDiscoveryProvider,
+        mapper,
+        configManager,
+        lookupCoordinatorManagerConfig
+    )
+    {
+      @Override
+      public Map<String, Map<String, LookupExtractorFactoryMapContainer>> getKnownLookups()
+      {
+        return ImmutableMap.of(LOOKUP_TIER, ImmutableMap.of(
+            "foo1", foo1,
+            "foo2", foo2
+        ));
+      }
+    };
+    manager.start();
+    final AuditInfo auditInfo = new AuditInfo("author", "comment", "localhost");
+    EasyMock.reset(configManager);
+    EasyMock.expect(
+        configManager.set(
+            EasyMock.eq(LookupCoordinatorManager.LOOKUP_CONFIG_KEY),
+            EasyMock.eq(
+                ImmutableMap.<String, Map<String, LookupExtractorFactoryMapContainer>>of()
+            ),
+            EasyMock.eq(auditInfo)
+        )
+    ).andReturn(SetResult.ok()).once();
+    EasyMock.replay(configManager);
+    Assert.assertTrue(manager.deleteTier(LOOKUP_TIER, auditInfo));
+    EasyMock.verify(configManager);
+  }
+
+  @Test
   public void testDeleteLookup()
   {
     final LookupExtractorFactoryMapContainer ignore = new LookupExtractorFactoryMapContainer(
@@ -896,6 +942,47 @@ public class LookupCoordinatorManagerTest
     EasyMock.verify(configManager);
   }
 
+
+  @Test
+  public void testDeleteLastLookup()
+  {
+    final LookupExtractorFactoryMapContainer lookup = new LookupExtractorFactoryMapContainer(
+        "v0",
+        ImmutableMap.of("lookup", "foo")
+    );
+    final LookupCoordinatorManager manager = new LookupCoordinatorManager(
+        client,
+        druidNodeDiscoveryProvider,
+        mapper,
+        configManager,
+        lookupCoordinatorManagerConfig
+    )
+    {
+      @Override
+      public Map<String, Map<String, LookupExtractorFactoryMapContainer>> getKnownLookups()
+      {
+        return ImmutableMap.of(LOOKUP_TIER, ImmutableMap.of(
+            "foo", lookup
+        ));
+      }
+    };
+    manager.start();
+    final AuditInfo auditInfo = new AuditInfo("author", "comment", "localhost");
+    EasyMock.reset(configManager);
+    EasyMock.expect(
+        configManager.set(
+            EasyMock.eq(LookupCoordinatorManager.LOOKUP_CONFIG_KEY),
+            EasyMock.eq(
+                ImmutableMap.<String, Map<String, LookupExtractorFactoryMapContainer>>of()
+            ),
+            EasyMock.eq(auditInfo)
+        )
+    ).andReturn(SetResult.ok()).once();
+    EasyMock.replay(configManager);
+    Assert.assertTrue(manager.deleteLookup(LOOKUP_TIER, "foo", auditInfo));
+    EasyMock.verify(configManager);
+  }
+
   @Test
   public void testDeleteLookupIgnoresMissing()
   {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org