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 2022/12/03 10:08:22 UTC

[GitHub] [pulsar] yuruguo opened a new pull request, #18728: [fix][broker] The interval of scheduled task should be greater than 0

yuruguo opened a new pull request, #18728:
URL: https://github.com/apache/pulsar/pull/18728

   ### Motivation
   Some scheduled tasks will be started when the broker starting, including:
   ```java
           // start other housekeeping functions
           this.startStatsUpdater(
                   serviceConfig.getStatsUpdateInitialDelayInSecs(),
                   serviceConfig.getStatsUpdateFrequencyInSecs());
           this.startInactivityMonitor();
           this.startMessageExpiryMonitor();
           this.startCompactionMonitor();
           this.startConsumedLedgersMonitor();
           this.startBacklogQuotaChecker();
           this.startCheckReplicationPolicies();
           this.startDeduplicationSnapshotMonitor();
   ```
   The interval of these scheduled tasks must be greater than 0, or an error will be reported during startup, as below:
   ```
   java.lang.IllegalArgumentException: null
   	at java.util.concurrent.ScheduledThreadPoolExecutor.scheduleAtFixedRate(ScheduledThreadPoolExecutor.java:623) ~[?:?]
   	at java.util.concurrent.Executors$DelegatedScheduledExecutorService.scheduleAtFixedRate(Executors.java:819) ~[?:?]
   	at org.apache.pulsar.broker.service.BrokerService.startInactivityMonitor(BrokerService.java:568) ~[classes/:?]
   	at org.apache.pulsar.broker.service.BrokerService.start(BrokerService.java:534) ~[classes/:?]
   	at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:784) ~[classes/:?]
   ```
   Therefore, it is necessary to judge whether the interval is greater than 0, and these scheduled tasks will be started only when the interval is greater than 0
   ### Modifications
   - check interval > 0
   
   ### Documentation
   - [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   PR in forked repository: 


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] codecov-commenter commented on pull request #18728: [fix][broker] The interval of scheduled task should be greater than 0

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #18728:
URL: https://github.com/apache/pulsar/pull/18728#issuecomment-1336136487

   # [Codecov](https://codecov.io/gh/apache/pulsar/pull/18728?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18728](https://codecov.io/gh/apache/pulsar/pull/18728?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9148a13) into [master](https://codecov.io/gh/apache/pulsar/commit/b579bb845b28c1404b75a76c44cc8beb74b2fcc3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b579bb8) will **decrease** coverage by `10.38%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pulsar/pull/18728/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pulsar/pull/18728?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #18728       +/-   ##
   =============================================
   - Coverage     47.51%   37.13%   -10.39%     
   + Complexity    10520     1971     -8549     
   =============================================
     Files           698      209      -489     
     Lines         68079    14421    -53658     
     Branches       7280     1573     -5707     
   =============================================
   - Hits          32351     5355    -26996     
   + Misses        32151     8480    -23671     
   + Partials       3577      586     -2991     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `37.13% <ø> (-10.39%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/18728?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...va/org/apache/pulsar/client/impl/ProducerBase.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1Byb2R1Y2VyQmFzZS5qYXZh) | `32.69% <0.00%> (-1.93%)` | :arrow_down: |
   | [...va/org/apache/pulsar/client/impl/ProducerImpl.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1Byb2R1Y2VySW1wbC5qYXZh) | `17.00% <0.00%> (-0.02%)` | :arrow_down: |
   | [...pulsar/broker/admin/impl/PersistentTopicsBase.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi9pbXBsL1BlcnNpc3RlbnRUb3BpY3NCYXNlLmphdmE=) | | |
   | [...loadbalance/impl/LeastResourceUsageWithWeight.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9sb2FkYmFsYW5jZS9pbXBsL0xlYXN0UmVzb3VyY2VVc2FnZVdpdGhXZWlnaHQuamF2YQ==) | | |
   | [...roker/loadbalance/impl/ModularLoadManagerImpl.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9sb2FkYmFsYW5jZS9pbXBsL01vZHVsYXJMb2FkTWFuYWdlckltcGwuamF2YQ==) | | |
   | [...lsar/broker/loadbalance/impl/ThresholdShedder.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9sb2FkYmFsYW5jZS9pbXBsL1RocmVzaG9sZFNoZWRkZXIuamF2YQ==) | | |
   | [.../pulsar/broker/service/AbstractBaseDispatcher.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL0Fic3RyYWN0QmFzZURpc3BhdGNoZXIuamF2YQ==) | | |
   | [...rg/apache/pulsar/broker/service/AbstractTopic.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL0Fic3RyYWN0VG9waWMuamF2YQ==) | | |
   | [...rg/apache/pulsar/broker/service/BrokerService.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL0Jyb2tlclNlcnZpY2UuamF2YQ==) | | |
   | [...tent/PersistentDispatcherSingleActiveConsumer.java](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL3BlcnNpc3RlbnQvUGVyc2lzdGVudERpc3BhdGNoZXJTaW5nbGVBY3RpdmVDb25zdW1lci5qYXZh) | | |
   | ... and [482 more](https://codecov.io/gh/apache/pulsar/pull/18728/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] yuruguo commented on pull request #18728: [fix][broker] The interval of scheduled task should be greater than 0

Posted by GitBox <gi...@apache.org>.
yuruguo commented on PR #18728:
URL: https://github.com/apache/pulsar/pull/18728#issuecomment-1336326211

   > I would prefer to let the broker fail with meaningful errors
   
   +1
   
   The `minValue = 1` has been added for verification on the `interval` field
   https://github.com/apache/pulsar/blob/3df506de099f27d4c3b7fbf2064db881ffda0402/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java#L1817-L1820
   and a clear message will be given and the broker will exit If interval < 1.
   


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] eolivelli merged pull request #18728: [improve][broker] The interval of scheduled task should be greater than 0

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


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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