You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by "AnonHxy (via GitHub)" <gi...@apache.org> on 2023/06/29 04:05:46 UTC
[GitHub] [bookkeeper] AnonHxy opened a new pull request, #4005: Optimize getActiveLedgersInRange
AnonHxy opened a new pull request, #4005:
URL: https://github.com/apache/bookkeeper/pull/4005
Descriptions of the changes in this PR:
### Motivation
Optimize `getActiveLedgersInRange(0, Long.MAX_VALUE)` by avoid Iterating the `ledgers`.
There are many places call `getActiveLedgersInRange(0, Long.MAX_VALUE)` to get all the `ledgerId`, for example:
1. https://github.com/apache/bookkeeper/blob/147b5bf846189584d6a9b4e20ac9a73e6e617255/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java#L124-L125
2. https://github.com/apache/bookkeeper/blob/147b5bf846189584d6a9b4e20ac9a73e6e617255/bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookie/ConvertToDBStorageCommand.java#L88
3. https://github.com/apache/bookkeeper/blob/147b5bf846189584d6a9b4e20ac9a73e6e617255/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LocationsIndexRebuildOp.java#L160
We could return the keys of `ledgers` directly to avoid Iterating the `ledgers`
### Changes
(Describe: what changes you have made)
Master Issue: #<master-issue-number>
> ---
> In order to uphold a high standard for quality for code contributions, Apache BookKeeper runs various precommit
> checks for pull requests. A pull request can only be merged when it passes precommit checks.
>
> ---
> Be sure to do all of the following to help us incorporate your contribution
> quickly and easily:
>
> If this PR is a BookKeeper Proposal (BP):
>
> - [ ] Make sure the PR title is formatted like:
> `<BP-#>: Description of bookkeeper proposal`
> `e.g. BP-1: 64 bits ledger is support`
> - [ ] Attach the master issue link in the description of this PR.
> - [ ] Attach the google doc link if the BP is written in Google Doc.
>
> Otherwise:
>
> - [ ] Make sure the PR title is formatted like:
> `<Issue #>: Description of pull request`
> `e.g. Issue 123: Description ...`
> - [ ] Make sure tests pass via `mvn clean apache-rat:check install spotbugs:check`.
> - [ ] Replace `<Issue #>` in the title with the actual Issue number.
>
> ---
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] horizonzy commented on pull request #4005: Optimize getActiveLedgersInRange
Posted by "horizonzy (via GitHub)" <gi...@apache.org>.
horizonzy commented on PR #4005:
URL: https://github.com/apache/bookkeeper/pull/4005#issuecomment-1617621446
That's fine.
------------------ Original ------------------
From: Hang Chen ***@***.***>
Date: Mon,Jul 3,2023 3:57 PM
To: apache/bookkeeper ***@***.***>
Cc: Yan Zhao ***@***.***>, Mention ***@***.***>
Subject: Re: [apache/bookkeeper] Optimize getActiveLedgersInRange (PR #4005)
We would better add a new method getAllActiveLedgers.
@horizonzy This change is low risk, and I think it's ok to be put in the original method.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #4005: Optimize getActiveLedgersInRange
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #4005:
URL: https://github.com/apache/bookkeeper/pull/4005#issuecomment-1617569458
> We would better add a new method `getAllActiveLedgers`.
@horizonzy This change is low risk, and I think it's ok to be put in the original method.
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] StevenLuMT commented on pull request #4005: Optimize getActiveLedgersInRange
Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT commented on PR #4005:
URL: https://github.com/apache/bookkeeper/pull/4005#issuecomment-1617458240
> We would better add a new method `getAllActiveLedgers`.
Whether to change the semantics of the original method, if it is changed, it is recommended to initiate a BP to add
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] StevenLuMT merged pull request #4005: Optimize getActiveLedgersInRange
Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT merged PR #4005:
URL: https://github.com/apache/bookkeeper/pull/4005
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org