You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/06/23 04:01:52 UTC

[GitHub] [pulsar] hangc0276 opened a new pull request #11028: fix flaky test in AdminApiOffloadTest

hangc0276 opened a new pull request #11028:
URL: https://github.com/apache/pulsar/pull/11028


   ### Motivation
   Fix flaky test in `AdminApiOffloadTest`. 
   When namespace level offload policy and topic level offload policy both not set, the `offloadPolicies` filed will be null.
   ```Java
   managedLedgerConfig.setLedgerOffloader(pulsar.getManagedLedgerOffloader(namespace, offloadPolicies));
   ```
   ```Java
   public LedgerOffloader getManagedLedgerOffloader(NamespaceName namespaceName, OffloadPoliciesImpl offloadPolicies) {
           if (offloadPolicies == null) {
               return getDefaultOffloader();
           }
           return ledgerOffloaderMap.compute(namespaceName, (ns, offloader) -> {
               try {
                   if (offloader != null && Objects.equals(offloader.getOffloadPolicies(), offloadPolicies)) {
                       return offloader;
                   } else {
                       if (offloader != null) {
                           offloader.close();
                       }
                       return createManagedLedgerOffloader(offloadPolicies);
                   }
               } catch (PulsarServerException e) {
                   LOG.error("create ledgerOffloader failed for namespace {}", namespaceName.toString(), e);
                   return new NullLedgerOffloader();
               }
           });
       }
   ```
   Then in `getManagedLedgerOffloader` method, it will return `getDefaultOffloader`. And the test will get the following exception.
   ```
   [ERROR] testTopicLevelOffloadNonPartitioned(org.apache.pulsar.broker.admin.AdminApiOffloadTest)  Time elapsed: 0.569 s  <<< FAILURE!
   java.lang.AssertionError: expected [s3] but found [NullLedgerOffloader]
   	at org.testng.Assert.fail(Assert.java:99)
   	at org.testng.Assert.failNotEquals(Assert.java:1037)
   	at org.testng.Assert.assertEqualsImpl(Assert.java:140)
   	at org.testng.Assert.assertEquals(Assert.java:122)
   	at org.testng.Assert.assertEquals(Assert.java:629)
   	at org.testng.Assert.assertEquals(Assert.java:639)
   	at org.apache.pulsar.broker.admin.AdminApiOffloadTest.testOffload(AdminApiOffloadTest.java:360)
   	at org.apache.pulsar.broker.admin.AdminApiOffloadTest.testTopicLevelOffloadNonPartitioned(AdminApiOffloadTest.java:267)
   	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.lang.reflect.Method.invoke(Method.java:498)
   	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
   	at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:45)
   	at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:73)
   	at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
   	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
   	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
   	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
   	at java.lang.Thread.run(Thread.java:748)
   
   ```
   
   ### Modification
   1. Add doReturn inspection for offloadPolicies is null.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] eolivelli merged pull request #11028: fix flaky test in AdminApiOffloadTest

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #11028:
URL: https://github.com/apache/pulsar/pull/11028


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on pull request #11028: fix flaky test in AdminApiOffloadTest

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11028:
URL: https://github.com/apache/pulsar/pull/11028#issuecomment-866509832


   > Why doesn't it appear every time?
   
   @315157973 because in current topic offload policy implementation, it doesn't refresh managedLedger configuration when topic offload policy updated, such as remote topic offload policy. When i fix the topic policy listener in #11021 , the test failed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] 315157973 commented on pull request #11028: fix flaky test in AdminApiOffloadTest

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #11028:
URL: https://github.com/apache/pulsar/pull/11028#issuecomment-866505373


   Why doesn't it appear every time?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org