[incubator-pulsar] branch branch-2.1 updated: Issue 2110: get-retention returns retentionSizeInMB as 0 when set to -1 (#2182)

[sijie]

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

sijie pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/incubator-pulsar.git


The following commit(s) were added to refs/heads/branch-2.1 by this push:
     new 406f8fa  Issue 2110: get-retention returns retentionSizeInMB as 0 when 
set to -1 (#2182)
406f8fa is described below

commit 406f8fa4de6d02839add4b50d31ef4d630b4c857
Author: Sijie Guo <guosi...@gmail.com>
AuthorDate: Wed Jul 25 23:02:25 2018 -0700

    Issue 2110: get-retention returns retentionSizeInMB as 0 when set to -1 
(#2182)
    
    * Issue 2110: get-retention returns retentionSizeInMB as 0 when set to -1
    
     ### Motivation
    
    Fixes #2110. infinite retention is not correctly propagated.
    
     ### Changes
    
    Fix the command tool to set infinite retention correctly. Update command 
line description and website to reflect the fact retention size less than 1MB 
is treated as no retention.
---
 .../org/apache/pulsar/admin/cli/CmdNamespaces.java | 10 ++++--
 site/_data/cli/pulsar-admin.yaml                   |  2 +-
 .../pulsar/tests/integration/cli/CLITest.java      | 38 ++++++++++++++++++++++
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git 
a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
 
b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
index 0eaf81b..4f898a5 100644
--- 
a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
+++ 
b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
@@ -342,7 +342,7 @@ public class CmdNamespaces extends CmdBase {
         private String retentionTimeStr;
 
         @Parameter(names = { "--size", "-s" }, description = "Retention size 
limit (eg: 10M, 16G, 3T). "
-                + "0 means no retention and -1 means infinite size retention", 
required = true)
+                + "0 or less than 1MB means no retention and -1 means infinite 
size retention", required = true)
         private String limitStr;
 
         @Override
@@ -351,8 +351,12 @@ public class CmdNamespaces extends CmdBase {
             long sizeLimit = validateSizeString(limitStr);
             int retentionTimeInMin = validateTimeString(retentionTimeStr);
 
-            sizeLimit = sizeLimit / (1024 * 1024);
-            int retentionSizeInMB = (int) sizeLimit;
+            int retentionSizeInMB;
+            if (sizeLimit != -1) {
+                retentionSizeInMB = (int) (sizeLimit / (1024 * 1024));
+            } else {
+                retentionSizeInMB = -1;
+            }
             admin.namespaces().setRetention(namespace, new 
RetentionPolicies(retentionTimeInMin, retentionSizeInMB));
         }
     }
diff --git a/site/_data/cli/pulsar-admin.yaml b/site/_data/cli/pulsar-admin.yaml
index c0d483f..94ec02c 100644
--- a/site/_data/cli/pulsar-admin.yaml
+++ b/site/_data/cli/pulsar-admin.yaml
@@ -431,7 +431,7 @@ commands:
     argument: tenant/cluster/namespace
     options:
     - flags: -s, --size
-      description: The retention size limits (for example `10M`, `16G` or 
`3T`). 0 means no retention and -1 means infinite size retention
+      description: The retention size limits (for example `10M`, `16G` or 
`3T`). 0 or less than 1MB means no retention and -1 means infinite size 
retention
     - flags: -t, --time
       description: "The retention time in minutes, hours, days, or weeks. 
Examples: `100m`, `13h`, `2d`, `5w`. 0 means no retention and -1 means infinite 
time retention"
   - name: unload
diff --git 
a/tests/integration/semantics/src/test/java/org/apache/pulsar/tests/integration/cli/CLITest.java
 
b/tests/integration/semantics/src/test/java/org/apache/pulsar/tests/integration/cli/CLITest.java
index 8d9ed20..0f2163a 100644
--- 
a/tests/integration/semantics/src/test/java/org/apache/pulsar/tests/integration/cli/CLITest.java
+++ 
b/tests/integration/semantics/src/test/java/org/apache/pulsar/tests/integration/cli/CLITest.java
@@ -164,4 +164,42 @@ public class CLITest extends PulsarClusterTestBase {
         );
         assertTrue(result.getStderr().contains("Reason: HTTP 404 Not Found"));
     }
+
+    @Test
+    public void testSetInfiniteRetention() throws Exception {
+        ContainerExecResult result;
+
+        String namespace = "get-and-set-retention" + randomName(8);
+        pulsarCluster.createNamespace(namespace);
+
+        String[] setCommand = {
+            "namespaces", "set-retention", "public/" + namespace,
+            "--size", "-1",
+            "--time", "-1"
+        };
+
+        result = pulsarCluster.runAdminCommandOnAnyBroker(setCommand);
+        assertEquals(0, result.getExitCode());
+        assertTrue(
+            result.getStdout().isEmpty(),
+            result.getStdout()
+        );
+        assertTrue(
+            result.getStderr().isEmpty(),
+            result.getStdout()
+        );
+
+        String[] getCommand = {
+            "namespaces", "get-retention", "public/" + namespace
+        };
+
+        result = pulsarCluster.runAdminCommandOnAnyBroker(getCommand);
+        assertEquals(0, result.getExitCode());
+        assertTrue(
+            result.getStdout().contains("\"retentionTimeInMinutes\" : -1"),
+            result.getStdout());
+        assertTrue(
+            result.getStdout().contains("\"retentionSizeInMB\" : -1"),
+            result.getStdout());
+    }
 }