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 2020/06/30 03:24:17 UTC

[GitHub] [pulsar] gaoran10 opened a new pull request #7389: [Offload] Set default value for offload-deletion-lag

gaoran10 opened a new pull request #7389:
URL: https://github.com/apache/pulsar/pull/7389


   Fixes #7347 
   
   ### Motivation
   
   The default value of the offload-deletion-lag is `null`, this will cause an NPE problem.
   
   ### Modifications
   
   Set the default value of the offload-deletion-lag to `-1`.
   
   ### Verifying this change
   
   Add unit test for method `ManagedLedgerImpl.isOffloadedNeedsDelete`.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API: (no)
     - The schema: (no)
     - The default values of configurations: (yes)
     - The wire protocol: (no)
     - The rest endpoints: (no)
     - The admin cli options: (no)
     - Anything that affects deployment: (no)
   
   ### Documentation
   
     - Does this pull request introduce a new feature? (no)
   


----------------------------------------------------------------
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] gaoran10 commented on pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

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


   JavaInstanceTest


----------------------------------------------------------------
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] codelipenghui commented on pull request #7389: [Issue 7347] Set default value for offload-deletion-lag

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] gaoran10 removed a comment on pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

Posted by GitBox <gi...@apache.org>.
gaoran10 removed a comment on pull request #7389:
URL: https://github.com/apache/pulsar/pull/7389#issuecomment-654071193


   JavaInstanceTest


----------------------------------------------------------------
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] gaoran10 commented on pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] gaoran10 commented on pull request #7389: [Issue 7347] Set default value for offload-deletion-lag

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] lukestephenson-zendesk commented on pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

Posted by GitBox <gi...@apache.org>.
lukestephenson-zendesk commented on pull request #7389:
URL: https://github.com/apache/pulsar/pull/7389#issuecomment-658788970


   I'm trying to work around this until 2.6.1 is released.  Not sure what to do.
   
   > The default value of the offload-deletion-lag is null, this will cause an NPE problem
   
   From the docs I thought the default value would be 14400000
   ![image](https://user-images.githubusercontent.com/48660846/87509585-35aafd00-c6b5-11ea-951d-be5ec77ac1ef.png)
   
   Will explicitly setting a value for this setting help?


----------------------------------------------------------------
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] gaoran10 removed a comment on pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

Posted by GitBox <gi...@apache.org>.
gaoran10 removed a comment on pull request #7389:
URL: https://github.com/apache/pulsar/pull/7389#issuecomment-654026068


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] sijie commented on pull request #7389: [Issue 7347] Set default value for offload-deletion-lag

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] gaoran10 commented on pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] gaoran10 commented on pull request #7389: [Issue 7347] Set default value for offload-deletion-lag

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] gaoran10 commented on pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] merlimat commented on a change in pull request #7389: [Issue 7347] Set default value for offload-deletion-lag

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #7389:
URL: https://github.com/apache/pulsar/pull/7389#discussion_r447399430



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/OffloadPolicies.java
##########
@@ -41,7 +41,7 @@
     public final static String[] DRIVER_NAMES = {"S3", "aws-s3", "google-cloud-storage", "filesystem"};
     public final static String DEFAULT_OFFLOADER_DIRECTORY = "./offloaders";
     public final static long DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES = -1;
-    public final static Long DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS = null;
+    public final static long DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS = -1;

Review comment:
       Should we also change `managedLedgerOffloadDeletionLagInMillis` to `long`? 




----------------------------------------------------------------
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] gaoran10 commented on a change in pull request #7389: [Issue 7347] Set default value for offload-deletion-lag

Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #7389:
URL: https://github.com/apache/pulsar/pull/7389#discussion_r448075017



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/OffloadPolicies.java
##########
@@ -41,7 +41,7 @@
     public final static String[] DRIVER_NAMES = {"S3", "aws-s3", "google-cloud-storage", "filesystem"};
     public final static String DEFAULT_OFFLOADER_DIRECTORY = "./offloaders";
     public final static long DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES = -1;
-    public final static Long DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS = null;
+    public final static long DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS = -1;

Review comment:
       Yes, I changed it. Thanks.




----------------------------------------------------------------
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] sijie merged pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

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


   


----------------------------------------------------------------
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] gaoran10 commented on pull request #7389: [Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffloadedNeedsDelete`

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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] sijie commented on pull request #7389: [Issue 7347] Set default value for offload-deletion-lag

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


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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