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/09/06 06:14:11 UTC

[GitHub] [pulsar] tjiuming opened a new pull request, #17485: [feat][tiered-storage] Introduce offload throttling

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

   Fixes https://github.com/apache/pulsar/issues/17362
   
   ### Motivation
   
    Support offload throttling, to prevent offloading use up all the broker resources(CPU, network).
   
    From the perspective of generality, we can't limit how many bytes write to tiered-storage, it will invade the corresponding implementations(filesystem, streaming, etc.). So we can only to limit how many bytes read from ML.
   
   ### Modifications
   
   1. Move `TimeWindow.java`、`WindowWrap.java`、`TimeWindowTest.java` from `broker` module to `managed-ledger` module.
   2. Add `managedLedgerGlobalOffloadingPermitBytesPerSecond` configuration, it works on the broker level.
   3. Add `globalOffloadingPermitBytesPerSecond` into `ManagedLedgerConfig`.
   
   ### 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: (yes)
     - 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)
   
   ### Documentation
   
   Check the box below or label this PR directly.
   
   Need to update docs? 
   
   - [x] `doc-required` 
   (Your PR needs to update docs and you will update later)
     
   - [ ] `doc-not-needed` 
   (Please explain why)
     
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)


-- 
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] PIP-211 [feat][tiered-storage] Introduce offload throttling, Part 1 [pulsar]

Posted by "dao-jun (via GitHub)" <gi...@apache.org>.
dao-jun closed pull request #17485: PIP-211 [feat][tiered-storage] Introduce offload throttling, Part 1
URL: https://github.com/apache/pulsar/pull/17485


-- 
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] tjiuming commented on pull request #17485: [feat][tiered-storage] Introduce offload throttling

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

   @hangc0276 @codelipenghui 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


Re: [PR] PIP-211 [feat][tiered-storage] Introduce offload throttling, Part 1 [pulsar]

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

   Replaced by https://github.com/apache/pulsar/pull/22385


-- 
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] poorbarcode commented on pull request #17485: PIP-211 [feat][tiered-storage] Introduce offload throttling, Part 1

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

   Since we will start the RC version of `3.0.0` on `2023-04-11`, I will change the label/milestone of PR who have not been merged.
   - The PR of type `feature` is deferred to `3.1.0`
   - The PR of type `fix` is deferred to `3.0.1`
   
   So drag this PR to `3.1.0`


-- 
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] zymap commented on a diff in pull request #17485: [feat][tiered-storage] Introduce offload throttling

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


##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -2972,7 +2973,7 @@ public void asyncOffloadPrefix(Position pos, OffloadCallback callback, Object ct
     }
 
     private void offloadLoop(CompletableFuture<PositionImpl> promise, Queue<LedgerInfo> ledgersToOffload,
-            PositionImpl firstUnoffloaded, Optional<Throwable> firstError) {
+                             PositionImpl firstUnoffloaded, Optional<Throwable> firstError) {

Review Comment:
   It's better to avoid changing the code format. Because we haven't a standard of that, everyone has their format. 
   It is also difficult to review if there has too much code format changes.



##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -2972,7 +2973,7 @@ public void asyncOffloadPrefix(Position pos, OffloadCallback callback, Object ct
     }
 
     private void offloadLoop(CompletableFuture<PositionImpl> promise, Queue<LedgerInfo> ledgersToOffload,
-            PositionImpl firstUnoffloaded, Optional<Throwable> firstError) {
+                             PositionImpl firstUnoffloaded, Optional<Throwable> firstError) {

Review Comment:
   It's better to avoid changing the code format. Because we haven't a standard of that, everyone has their format. 
   It is also difficult to review if there have too many code format changes.



-- 
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] tjiuming commented on pull request #17485: [feat][tiered-storage] Introduce offload throttling

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

   I'll create another PR to support restApi/cmd to modify offload policies after the PR merged


-- 
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] github-actions[bot] commented on pull request #17485: [feat][tiered-storage] Introduce offload throttling

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17485:
URL: https://github.com/apache/pulsar/pull/17485#issuecomment-1284820788

   The pr had no activity for 30 days, mark with Stale label.


-- 
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 pull request #17485: [feat][tiered-storage] Introduce offload throttling

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

   @tjiuming, Could you please create a proposal for this one? We will introduce a new mechanism, we need to consider how to operate and monitor the new feature.


-- 
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] github-actions[bot] commented on pull request #17485: PIP-211 [feat][tiered-storage] Introduce offload throttling, Part 1

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17485:
URL: https://github.com/apache/pulsar/pull/17485#issuecomment-1544991655

   The pr had no activity for 30 days, mark with Stale label.


-- 
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] tjiuming commented on pull request #17485: [feat][tiered-storage] Introduce offload throttling

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

   > @tjiuming, Could you please create a proposal for this one? We will introduce a new mechanism, we need to consider how to operate and monitor the new feature.
   
   @codelipenghui  Yes, I'm re-designing the flow-controller, the current implementation will block read thread and will take effect to other ledger's reading. After that, I'll open a proposal


-- 
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] tjiuming commented on pull request #17485: [feat][tiered-storage] Introduce offload throttling

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

   Tests to be completed.


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