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/10/20 14:30:44 UTC
[GitHub] [pulsar] gaoran10 opened a new pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
gaoran10 opened a new pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310
Fixes #8220
### Motivation
Currently, the fields' values couldn't be set properly by the Pulsar broker config file(`broker.conf` or `standalone.conf`).
### Modifications
changed fields in `OffloadPolicies`
1. managedLedgerOffloadAutoTriggerSizeThresholdBytes
2. managedLedgerOffloadDeletionLagMs
### Verifying this change
This change added tests and can be verified as follows:
- *org.apache.pulsar.common.policies.data.OffloadPoliciesTest*
### 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: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-716886471
/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] jiazhai commented on pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-716427453
/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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-717635812
/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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-718886305
@milos-matijasevic Yes, the default value for the `managedLedgerOffloadDeletionLagMs` is 4 hours, it will use the broker configuration.
----------------------------------------------------------------
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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-717298710
/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 a change in pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#discussion_r510180729
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/OffloadPolicies.java
##########
@@ -51,8 +51,8 @@
private String managedLedgerOffloadDriver = null;
private int managedLedgerOffloadMaxThreads = DEFAULT_OFFLOAD_MAX_THREADS;
private int managedLedgerOffloadPrefetchRounds = DEFAULT_OFFLOAD_MAX_PREFETCH_ROUNDS;
- private long managedLedgerOffloadThresholdInBytes = DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES;
- private Long managedLedgerOffloadDeletionLagInMillis = DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS;
+ private long managedLedgerOffloadAutoTriggerSizeThresholdBytes = DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES;
Review comment:
Yes, this change will cause the compatible problem, I resolved it.
----------------------------------------------------------------
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 a change in pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#discussion_r508996045
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/OffloadPolicies.java
##########
@@ -51,8 +51,8 @@
private String managedLedgerOffloadDriver = null;
private int managedLedgerOffloadMaxThreads = DEFAULT_OFFLOAD_MAX_THREADS;
private int managedLedgerOffloadPrefetchRounds = DEFAULT_OFFLOAD_MAX_PREFETCH_ROUNDS;
- private long managedLedgerOffloadThresholdInBytes = DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES;
- private Long managedLedgerOffloadDeletionLagInMillis = DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS;
+ private long managedLedgerOffloadAutoTriggerSizeThresholdBytes = DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES;
Review comment:
What is the backward compatibility story here because we are changing the field name for `OffloadPolicies`? If there is already a namespace policy written using previous field names, how do we guarantee the new code is able to read the offload policies written using the old code?
----------------------------------------------------------------
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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 removed a comment on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-717092260
/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] codelipenghui merged pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310
----------------------------------------------------------------
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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#discussion_r510180729
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/OffloadPolicies.java
##########
@@ -51,8 +51,8 @@
private String managedLedgerOffloadDriver = null;
private int managedLedgerOffloadMaxThreads = DEFAULT_OFFLOAD_MAX_THREADS;
private int managedLedgerOffloadPrefetchRounds = DEFAULT_OFFLOAD_MAX_PREFETCH_ROUNDS;
- private long managedLedgerOffloadThresholdInBytes = DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES;
- private Long managedLedgerOffloadDeletionLagInMillis = DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS;
+ private long managedLedgerOffloadAutoTriggerSizeThresholdBytes = DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES;
Review comment:
Yes, this change will cause the compatible problem, I resolved this problem.
----------------------------------------------------------------
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] jiazhai commented on pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-713239420
@gaoran10 seems the `CI - Misc / License check (pull_request)` has a issue
----------------------------------------------------------------
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] wolfstudy commented on pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-716949826
@sijie PTAL again, 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] codelipenghui commented on pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-713235652
/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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-717092260
/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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-714429035
/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] jiazhai commented on pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-716905141
/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] codelipenghui commented on pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-713264238
/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] milos-matijasevic commented on pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
milos-matijasevic commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-718868963
Hey guys, i have one question, after this what is default value for `managedLedgerOffloadDeletionLagMs` 4 hours or null(never).
In code it was null but now if it's read from broker config ([broker.conf](https://github.com/apache/pulsar/blob/master/conf/broker.conf#L817)) it will be 4 hours ?
----------------------------------------------------------------
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 edited a comment on pull request #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
gaoran10 edited a comment on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-718886305
@milos-matijasevic Yes, the default value for the `managedLedgerOffloadDeletionLagMs` is 4 hours, it will use the broker configuration unless you change the broker configuration or change the namespace level config `managedLedgerOffloadDeletionLagMs` through the command like this `pulsar-admin namespaces set-offload-deletion-lag -l 1d`.
----------------------------------------------------------------
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 #8310: [Offload] Make the field name in `OffloadPolicies` match with config file
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #8310:
URL: https://github.com/apache/pulsar/pull/8310#issuecomment-714838108
/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