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/10/28 10:01:10 UTC

[GitHub] [pulsar] Pomelongan opened a new pull request, #18244: [improve][broker]optimize the log printing logic of PulsarService

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

   ### Motivation
   In `org.apache.pulsar.broker.PulsarService#start`, two log printing can be combined into one line.
   https://github.com/apache/pulsar/blob/b7b923933a5c7594a52c8bb761f80e25ca0065cb/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java#L905-L907
   ### Modifications
   Combine the two log lines in the `org.apache.pulsar.broker.PulsarService#start` into one line.
   
   ### Verifying this change
   
   - [x] Make sure that the change passes the CI checks.
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If the box was checked, please highlight the changes*
   
   - [ ] Dependencies (add or upgrade a dependency)
   - [ ] The public API
   - [ ] The schema
   - [ ] The default values of configurations
   - [ ] The binary protocol
   - [ ] The REST endpoints
   - [ ] The admin CLI options
   - [ ] Anything that affects deployment
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes -->
   - [ ] `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: https://github.com/Pomelongan/pulsar/pull/11
   


-- 
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] AnonHxy commented on a diff in pull request #18244: [improve][broker]optimize the log printing logic of PulsarService

Posted by GitBox <gi...@apache.org>.
AnonHxy commented on code in PR #18244:
URL: https://github.com/apache/pulsar/pull/18244#discussion_r1007949471


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java:
##########
@@ -902,9 +902,8 @@ public void start() throws PulsarServerException {
                     + (config.getWebServicePortTls().isPresent() ? ", tls-port = " + config.getWebServicePortTls() : "")
                     + (StringUtils.isNotEmpty(brokerServiceUrl) ? ", broker url= " + brokerServiceUrl : "")
                     + (StringUtils.isNotEmpty(brokerServiceUrlTls) ? ", broker tls url= " + brokerServiceUrlTls : "");
-            LOG.info("messaging service is ready, bootstrap_seconds={}", bootstrapTimeSeconds);
-            LOG.info("messaging service is ready, {}, cluster={}, configs={}", bootstrapMessage,
-                    config.getClusterName(), config);
+            LOG.info("messaging service is ready, {}, cluster={}, configs={}, bootstrap_seconds={}", bootstrapMessage,

Review Comment:
   It's better move the `bootstrap_seconds` to  first position,  because the  `configs` will be very 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.

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

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


[GitHub] [pulsar] codelipenghui commented on a diff in pull request #18244: [improve][broker]optimize the log printing logic of PulsarService

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #18244:
URL: https://github.com/apache/pulsar/pull/18244#discussion_r1008036255


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java:
##########
@@ -902,9 +902,8 @@ public void start() throws PulsarServerException {
                     + (config.getWebServicePortTls().isPresent() ? ", tls-port = " + config.getWebServicePortTls() : "")
                     + (StringUtils.isNotEmpty(brokerServiceUrl) ? ", broker url= " + brokerServiceUrl : "")
                     + (StringUtils.isNotEmpty(brokerServiceUrlTls) ? ", broker tls url= " + brokerServiceUrlTls : "");
-            LOG.info("messaging service is ready, bootstrap_seconds={}", bootstrapTimeSeconds);
-            LOG.info("messaging service is ready, {}, cluster={}, configs={}", bootstrapMessage,
-                    config.getClusterName(), config);
+            LOG.info("messaging service is ready, {}, cluster={}, configs={}, bootstrap_seconds={}", bootstrapMessage,

Review Comment:
   +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] codecov-commenter commented on pull request #18244: [improve][broker] Optimize the log printing of PulsarService

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

   # [Codecov](https://codecov.io/gh/apache/pulsar/pull/18244?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 [#18244](https://codecov.io/gh/apache/pulsar/pull/18244?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (75e867a) into [master](https://codecov.io/gh/apache/pulsar/commit/46c0438ccc1460e22fc437a626a8c9cb05f4d618?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (46c0438) will **increase** coverage by `31.54%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pulsar/pull/18244/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/18244?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   #18244       +/-   ##
   =============================================
   + Coverage     38.84%   70.39%   +31.54%     
   + Complexity     8286      436     -7850     
   =============================================
     Files           683       26      -657     
     Lines         67323     2246    -65077     
     Branches       7215      245     -6970     
   =============================================
   - Hits          26152     1581    -24571     
   + Misses        38210      492    -37718     
   + Partials       2961      173     -2788     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `70.39% <ø> (+31.54%)` | :arrow_up: |
   
   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/18244?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...java/org/apache/pulsar/proxy/stats/TopicStats.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLXByb3h5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvcHJveHkvc3RhdHMvVG9waWNTdGF0cy5qYXZh) | `58.82% <0.00%> (-41.18%)` | :arrow_down: |
   | [...a/org/apache/pulsar/proxy/server/ProxyService.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLXByb3h5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvcHJveHkvc2VydmVyL1Byb3h5U2VydmljZS5qYXZh) | `80.00% <0.00%> (-0.94%)` | :arrow_down: |
   | [...rg/apache/pulsar/proxy/server/ProxyConnection.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLXByb3h5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvcHJveHkvc2VydmVyL1Byb3h5Q29ubmVjdGlvbi5qYXZh) | `55.21% <0.00%> (-0.29%)` | :arrow_down: |
   | [...n/java/org/apache/pulsar/broker/PulsarService.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9QdWxzYXJTZXJ2aWNlLmphdmE=) | | |
   | [.../buffer/impl/TransactionBufferClientStatsImpl.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci90cmFuc2FjdGlvbi9idWZmZXIvaW1wbC9UcmFuc2FjdGlvbkJ1ZmZlckNsaWVudFN0YXRzSW1wbC5qYXZh) | | |
   | [...ar/common/naming/PartitionedManagedLedgerInfo.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9uYW1pbmcvUGFydGl0aW9uZWRNYW5hZ2VkTGVkZ2VySW5mby5qYXZh) | | |
   | [...he/pulsar/broker/admin/v1/NonPersistentTopics.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9Ob25QZXJzaXN0ZW50VG9waWNzLmphdmE=) | | |
   | [...e/bookkeeper/mledger/impl/LedgerMetadataUtils.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL0xlZGdlck1ldGFkYXRhVXRpbHMuamF2YQ==) | | |
   | [...org/apache/pulsar/broker/admin/v2/BrokerStats.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92Mi9Ccm9rZXJTdGF0cy5qYXZh) | | |
   | [...ache/pulsar/client/impl/MultiTopicsReaderImpl.java](https://codecov.io/gh/apache/pulsar/pull/18244/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-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL011bHRpVG9waWNzUmVhZGVySW1wbC5qYXZh) | | |
   | ... and [650 more](https://codecov.io/gh/apache/pulsar/pull/18244/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] AnonHxy merged pull request #18244: [improve][broker] Optimize the log printing of PulsarService

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


-- 
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] Pomelongan commented on a diff in pull request #18244: [improve][broker] Optimize the log printing of PulsarService

Posted by GitBox <gi...@apache.org>.
Pomelongan commented on code in PR #18244:
URL: https://github.com/apache/pulsar/pull/18244#discussion_r1008664053


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java:
##########
@@ -902,9 +902,8 @@ public void start() throws PulsarServerException {
                     + (config.getWebServicePortTls().isPresent() ? ", tls-port = " + config.getWebServicePortTls() : "")
                     + (StringUtils.isNotEmpty(brokerServiceUrl) ? ", broker url= " + brokerServiceUrl : "")
                     + (StringUtils.isNotEmpty(brokerServiceUrlTls) ? ", broker tls url= " + brokerServiceUrlTls : "");
-            LOG.info("messaging service is ready, bootstrap_seconds={}", bootstrapTimeSeconds);
-            LOG.info("messaging service is ready, {}, cluster={}, configs={}", bootstrapMessage,
-                    config.getClusterName(), config);
+            LOG.info("messaging service is ready, {}, cluster={}, configs={}, bootstrap_seconds={}", bootstrapMessage,

Review Comment:
   @codelipenghui @AnonHxy I have updated, PTAL.



-- 
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] codelipenghui closed pull request #18244: [improve][broker] Optimize the log printing of PulsarService

Posted by GitBox <gi...@apache.org>.
codelipenghui closed pull request #18244: [improve][broker] Optimize the log printing of PulsarService
URL: https://github.com/apache/pulsar/pull/18244


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