You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by reddycharan <gi...@git.apache.org> on 2017/04/08 05:06:05 UTC
[GitHub] bookkeeper pull request #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
GitHub user reddycharan opened a pull request:
https://github.com/apache/bookkeeper/pull/127
BOOKKEEPER-1028 and BOOKKEEPER-1029
BOOKKEEPER-1028: inc/excl opts listunderreplicated
- Introduce including and excluding BookieId options
for listunderreplicatedLedgers
- But there is limitation that, since replicaslist wont be
updated in underreplicatedledger zNode there is possibility
of stale information
---------------------------------------------------------
BOOKKEEPER-1029: BookieDecommision Workflow
- LostBookieRecoveryDelay config param is stored in ZK
- if LostBookieRecoveryDelay is reset to same value then it force triggers audit immediately
- Added logic to trigger immediately or schedule pending audittask depending on the changed value in ZK
- good number of testcases validating focetrigger/reschedluing audittask
- added bookieshell command to get/set LostBookieRecoveryDelay from ZK
- added bookieshell command to triggeraudit by resetting LostBookieRecoveryDelay
- added decommissionbookie bkshell command, which validates the complete replication of ledgers stored in the bookie
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/reddycharan/bookkeeper listunderreplicatedpredicate
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/bookkeeper/pull/127.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #127
----
commit f4787383fbf7251dd82075c5b959342b137949c7
Author: cguttapalem <cg...@salesforce.com>
Date: 2016-10-14T06:32:39Z
BOOKKEEPER-1028: inc/excl opts listunderreplicated
- Introduce including and excluding BookieId options
for listunderreplicatedLedgers
- But there is limitation that, since replicaslist wont be
updated in underreplicatedledger zNode there is possibility
of stale information
commit 699cf56f4262d3a7d655bc64f00dded6bc814ee3
Author: Charan Reddy Guttapalem <cg...@salesforce.com>
Date: 2017-02-28T21:23:34Z
BOOKKEEPER-1029: BookieDecommision Workflow
- LostBookieRecoveryDelay config param is stored in ZK
- if LostBookieRecoveryDelay is reset to same value then it force triggers audit immediately
- Added logic to trigger immediately or schedule pending audittask depending on the changed value in ZK
- good number of testcases validating focetrigger/reschedluing audittask
- added bookieshell command to get/set LostBookieRecoveryDelay from ZK
- added bookieshell command to triggeraudit by resetting LostBookieRecoveryDelay
- added decommissionbookie bkshell command, which validates the complete replication of ledgers stored in the bookie
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by reddycharan <gi...@git.apache.org>.
Github user reddycharan commented on the issue:
https://github.com/apache/bookkeeper/pull/127
@sijie addressed your comments -
- move logic from BKShell to BookkeeperAdmin
- added functional testcases
- make DistributionSchedule and RoundRobinDistributionSchedule non-public
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by reddycharan <gi...@git.apache.org>.
Github user reddycharan commented on the issue:
https://github.com/apache/bookkeeper/pull/127
@sijie if you dont have anymore comments, this commit also should be good to go.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by reddycharan <gi...@git.apache.org>.
Github user reddycharan commented on the issue:
https://github.com/apache/bookkeeper/pull/127
@eolivelli @sijie
fixed rat check warnings you mentioned.
Yes 'decommissionbookie' bkshell command has to be executed in the node, which is being decommissioned.
added considerable number of comments and added clear description to the commit explaining about auditor trigger logic.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by reddycharan <gi...@git.apache.org>.
Github user reddycharan commented on the issue:
https://github.com/apache/bookkeeper/pull/127
@sijie my bad, missed checking findbugs error after previous push. Fixed them now and it passed.
Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:
https://github.com/apache/bookkeeper/pull/127
jenkins job crashed, I restarted it
https://builds.apache.org/job/bookkeeper-precommit-pullrequest/54/console
CI completed with 2 tests failures, I think they are not releated to the patch
Failed tests:
TestSpeculativeRead.testSpeculativeReadFirstReadCompleteIsOk:260
TestBackwardCompat.testCompat410:647 Shouldn't be able to write
Tests run: 1167, Failures: 2, Errors: 0, Skipped: 0
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:
https://github.com/apache/bookkeeper/pull/127
@reddycharan there is a findbugs error introduced in this pull request. Can you fix that?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:
https://github.com/apache/bookkeeper/pull/127
@reddycharan I have started a review.
All tests run without failures on my laptop (after rebasing to current master)
Apache Rat check is OK
But I see the findbugs errors below, can you fix them ?
Maybe you can rebase to current master, now the full test suite runs in 20 minutes
```
[INFO] Result of integer multiplication cast to long in org.apache.bookkeeper.bookie.BookieShell$DecommissionBookieCmd.waitForLedgersToBeReplicated(Collection, BookieSocketAddress, LedgerManager) [org.apache.bookkeeper.bookie.BookieShell$DecommissionBookieCmd] At BookieShell.java:[line 2081] ICAST_INTEGER_MULTIPLY_CAST_TO_LONG
[INFO] Boxing/unboxing to parse a primitive org.apache.bookkeeper.bookie.BookieShell$LostBookieRecoveryDelayCmd.runCmd(CommandLine) [org.apache.bookkeeper.bookie.BookieShell$LostBookieRecoveryDelayCmd] At BookieShell.java:[line 1403] DM_BOXED_PRIMITIVE_FOR_PARSING
[INFO] Boxing/unboxing to parse a primitive org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager.getLostBookieRecoveryDelay() [org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager] At ZkLedgerUnderreplicationManager.java:[line 741] DM_BOXED_PRIMITIVE_FOR_PARSING
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by reddycharan <gi...@git.apache.org>.
Github user reddycharan commented on the issue:
https://github.com/apache/bookkeeper/pull/127
@eolivelli I fixed it. It seems this time it has passed. Boxing/unboxing checks is new thing to me. Thanks for the follow up.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:
https://github.com/apache/bookkeeper/pull/127
I am sorry @reddycharan but findbugs is still failing
[INFO] Boxing/unboxing to parse a primitive org.apache.bookkeeper.bookie.BookieShell$LostBookieRecoveryDelayCmd.runCmd(CommandLine) [org.apache.bookkeeper.bookie.BookieShell$LostBookieRecoveryDelayCmd] At BookieShell.java:[line 1403] DM_BOXED_PRIMITIVE_FOR_PARSING
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:
https://github.com/apache/bookkeeper/pull/127
@reddycharan do you mind addressing the findbugs error?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #127: BOOKKEEPER-1028 and BOOKKEEPER-1029
Posted by reddycharan <gi...@git.apache.org>.
Github user reddycharan commented on the issue:
https://github.com/apache/bookkeeper/pull/127
sure, will do. I was just waiting for full code review and comments, so that I can address all of them at once.
Thanks,
Charan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---