You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "JooHyukKim (via GitHub)" <gi...@apache.org> on 2023/09/04 11:36:48 UTC

[GitHub] [pulsar] JooHyukKim opened a new pull request, #20663: [feat][cli] Add command line option for configuring the memory limit

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

   Fixes #15912
   
   ### Motivation
   
   > Cli tools such as pulsar-perf don't contain a way to configure the Pulsar client memory limit.
   
   As described in #15912 
   
   ### Modifications
   
   - Add `--memory-limit` configuration for `PerformanceBaseArguments`
   - Implement `MemoryUnitToByteConverter` implementing JCommander's `IStringConverter<Long>` for isolating parsing logic.
   - Took reference for parsing memory units (such as gigabytes etc...) from PR #20646
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   ### Does this pull request potentially affect one of the following parts:
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   *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 threading model
   - [ ] The binary protocol
   - [ ] The REST endpoints
   - [ ] The admin CLI options
   - [ ] The metrics
   - [ ] Anything that affects deployment
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. -->
   - [x] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [ ] `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/JooHyukKim/pulsar/pull/14
   
   


-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1627731023

   We might want to proceed with #20691, so we can get rid of converters added here. Or refactor after merging might do.


-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1741764242

   Converted to `draft PR` as we have couple of failures to fix.
   Anyone is welcome to pick up and go on.


-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1689989533

   ### Closing Note
   
   Temporarily closing due to PIP-280 work is taking longer than expected.
   Anyone can pick up where this PR left off, not much left to do, just import `pulsar-cli-utils` module.


-- 
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] JooHyukKim closed pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim closed pull request #20663: [feat][cli] Add command line option for configuring the memory limit
URL: https://github.com/apache/pulsar/pull/20663


-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1731245767

   [OWASP dependency workflow](https://github.com/apache/pulsar/actions/runs/6223359436/job/16926237362?pr=20764) failing here, and also in the [other PR](https://github.com/apache/pulsar/actions/runs/6220758680/job/16951596528?pr=20663). As a regular commiter, this many number of CVE's to fix is overwhelming 😭 


-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1609848119

   Apologies, my mistake 🙏🏼. I was suppposed to open PR in my personal repo first.


-- 
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] tisonkun commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1731396883

   @JooHyukKim If the OWASP failures are unrelated to this patch, it's not a blocker. Don't worry.
   
   I just too busy to give a review 🤣 


-- 
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] gaoran10 commented on a diff in pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "gaoran10 (via GitHub)" <gi...@apache.org>.
gaoran10 commented on code in PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#discussion_r1244640799


##########
pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/PulsarClientTool.java:
##########
@@ -76,6 +76,9 @@ public static class RootParams {
 
         @Parameter(names = { "--tlsTrustCertsFilePath" }, description = "File path to client trust certificates")
         String tlsTrustCertsFilePath;
+
+        @Parameter(names = { "-m", "--memory", }, description = "Configure the Pulsar client memory limit")

Review Comment:
   Could we make this consistent with the performance commands?
   
   Such as
   ```
   @Parameter(names = { "-ml", "--memory-limit", }, description = "Configure the Pulsar client memory limit "
               + "(eg: 32M, 64M)")
   ```



-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1731438958

   /pulsarbot rerun-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.

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

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

Re: [PR] [feat][cli] Add command line option for configuring the memory limit [pulsar]

Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun merged PR #20663:
URL: https://github.com/apache/pulsar/pull/20663


-- 
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] JooHyukKim commented on a diff in pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on code in PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#discussion_r1245000532


##########
pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/PulsarClientTool.java:
##########
@@ -76,6 +76,9 @@ public static class RootParams {
 
         @Parameter(names = { "--tlsTrustCertsFilePath" }, description = "File path to client trust certificates")
         String tlsTrustCertsFilePath;
+
+        @Parameter(names = { "-m", "--memory", }, description = "Configure the Pulsar client memory limit")

Review Comment:
   @gaoran10 Yes for sure! I was going to actually. Thank you for the reminder tho 🙏🏼 



-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1637051151

   Let's wait til #20782 gets merged so we can make use of `Converter` utils.


-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1646568502

   Blocked by #20782 


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

Re: [PR] [feat][cli] Add command line option for configuring the memory limit [pulsar]

Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1754623186

   Let me invesitgate the license 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.

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

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

[GitHub] [pulsar] JooHyukKim commented on a diff in pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on code in PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#discussion_r1245268820


##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/LoadSimulationClient.java:
##########
@@ -180,6 +180,10 @@ private static class MainArguments {
 
         @Parameter(names = { "--service-url" }, description = "Pulsar Service URL", required = true)
         public String serviceURL;
+
+        @Parameter(names = { "-ml", "--memory-limit", }, description = "Configure the Pulsar client memory limit "
+            + "(eg: 32M, 64M)", converter = MemoryUnitToByteConverter.class)
+        public long memoryLimit = 0L;

Review Comment:
   I think this one we might be able to utilize over at PR #20646 



-- 
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] JooHyukKim commented on pull request #20663: [feat][cli] Add command line option for configuring the memory limit

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1731439742

   > I just too busy to give a review 
   
   Right, thanks always! 🙏🏽🙏🏽


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

Re: [PR] [feat][cli] Add command line option for configuring the memory limit [pulsar]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#issuecomment-1754986307

   ## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#20663](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (dc5eed4) into [master](https://app.codecov.io/gh/apache/pulsar/commit/8438e43fb02b6d8df29672c6460a2f3da924ab60?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (8438e43) will **increase** coverage by `36.35%`.
   > Report is 7 commits behind head on master.
   > The diff coverage is `86.20%`.
   
   [![Impacted file tree graph](https://app.codecov.io/gh/apache/pulsar/pull/20663/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=apache)](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #20663       +/-   ##
   =============================================
   + Coverage     36.84%   73.20%   +36.35%     
   - Complexity    12229    32500    +20271     
   =============================================
     Files          1699     1888      +189     
     Lines        130559   140251     +9692     
     Branches      14264    15444     +1180     
   =============================================
   + Hits          48106   102670    +54564     
   + Misses        76121    29480    -46641     
   - Partials       6332     8101     +1769     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pulsar/pull/20663/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [inttests](https://app.codecov.io/gh/apache/pulsar/pull/20663/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.14% <48.27%> (-0.19%)` | :arrow_down: |
   | [systests](https://app.codecov.io/gh/apache/pulsar/pull/20663/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.67% <37.93%> (-0.06%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pulsar/pull/20663/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.50% <86.20%> (+40.55%)` | :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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Files](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [...org/apache/bookkeeper/mledger/LedgerOffloader.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9MZWRnZXJPZmZsb2FkZXIuamF2YQ==) | `72.22% <ø> (+72.22%)` | :arrow_up: |
   | [...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL01hbmFnZWRMZWRnZXJJbXBsLmphdmE=) | `80.63% <100.00%> (+32.31%)` | :arrow_up: |
   | [...e/bookkeeper/mledger/impl/NullLedgerOffloader.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL051bGxMZWRnZXJPZmZsb2FkZXIuamF2YQ==) | `35.71% <ø> (+7.14%)` | :arrow_up: |
   | [...areness/IsolatedBookieEnsemblePlacementPolicy.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9ib29raWUvcmFja2F3YXJlbmVzcy9Jc29sYXRlZEJvb2tpZUVuc2VtYmxlUGxhY2VtZW50UG9saWN5LmphdmE=) | `87.02% <100.00%> (+87.02%)` | :arrow_up: |
   | [...va/org/apache/pulsar/broker/service/ServerCnx.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1NlcnZlckNueC5qYXZh) | `71.81% <100.00%> (+30.29%)` | :arrow_up: |
   | [...n/java/org/apache/pulsar/broker/service/Topic.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1RvcGljLmphdmE=) | `36.36% <ø> (+28.03%)` | :arrow_up: |
   | [...org/apache/pulsar/client/cli/PulsarClientTool.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC10b29scy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9jbGkvUHVsc2FyQ2xpZW50VG9vbC5qYXZh) | `68.45% <100.00%> (+12.90%)` | :arrow_up: |
   | [...esystem/impl/FileSystemManagedLedgerOffloader.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dGllcmVkLXN0b3JhZ2UvZmlsZS1zeXN0ZW0vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9vZmZsb2FkL2ZpbGVzeXN0ZW0vaW1wbC9GaWxlU3lzdGVtTWFuYWdlZExlZGdlck9mZmxvYWRlci5qYXZh) | `68.04% <ø> (ø)` | |
   | [...d/jcloud/impl/BlobStoreManagedLedgerOffloader.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dGllcmVkLXN0b3JhZ2UvamNsb3VkL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ib29ra2VlcGVyL21sZWRnZXIvb2ZmbG9hZC9qY2xvdWQvaW1wbC9CbG9iU3RvcmVNYW5hZ2VkTGVkZ2VyT2ZmbG9hZGVyLmphdmE=) | `75.18% <ø> (ø)` | |
   | [...ava/org/apache/pulsar/broker/service/Producer.java](https://app.codecov.io/gh/apache/pulsar/pull/20663?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1Byb2R1Y2VyLmphdmE=) | `82.09% <75.00%> (+19.66%)` | :arrow_up: |
   
   ... and [1441 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/20663/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   


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

Re: [PR] [feat][cli] Add command line option for configuring the memory limit [pulsar]

Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on code in PR #20663:
URL: https://github.com/apache/pulsar/pull/20663#discussion_r1352307211


##########
src/check-binary-license.sh:
##########
@@ -41,7 +41,7 @@ if [ -z $TARBALL ]; then
     exit 1
 fi
 
-JARS=$(tar -tf $TARBALL | grep '\.jar' | grep -v 'trino/' | grep -v '/examples/' | grep -v '/instances/' | grep -v pulsar-client | grep -v pulsar-common | grep -v pulsar-package | grep -v pulsar-websocket | grep -v bouncy-castle-bc | sed 's!.*/!!' | sort)
+JARS=$(tar -tf $TARBALL | grep '\.jar' | grep -v 'trino/' | grep -v '/examples/' | grep -v '/instances/' | grep -v pulsar-client | grep -v pulsar-cli-utils | grep -v pulsar-common | grep -v pulsar-package | grep -v pulsar-websocket | grep -v bouncy-castle-bc | sed 's!.*/!!' | sort)

Review Comment:
   Oh... this was it. Thank you @tisonkun ! 😭



##########
src/check-binary-license.sh:
##########
@@ -41,7 +41,7 @@ if [ -z $TARBALL ]; then
     exit 1
 fi
 
-JARS=$(tar -tf $TARBALL | grep '\.jar' | grep -v 'trino/' | grep -v '/examples/' | grep -v '/instances/' | grep -v pulsar-client | grep -v pulsar-common | grep -v pulsar-package | grep -v pulsar-websocket | grep -v bouncy-castle-bc | sed 's!.*/!!' | sort)
+JARS=$(tar -tf $TARBALL | grep '\.jar' | grep -v 'trino/' | grep -v '/examples/' | grep -v '/instances/' | grep -v pulsar-client | grep -v pulsar-cli-utils | grep -v pulsar-common | grep -v pulsar-package | grep -v pulsar-websocket | grep -v bouncy-castle-bc | sed 's!.*/!!' | sort)

Review Comment:
   Oh... this was it. Thank you @tisonkun ! 😭



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