sijie commented on a change in pull request #641: Issue-596 Issue-583: Auto
replication should honor ensemble placement policy
URL: https://github.com/apache/bookkeeper/pull/641#discussion_r146480717
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
##########
@@ -672,55 +619,52 @@ public void process(Long ledgerId,
AsyncCallback.VoidCallback iterCallback) {
}
/**
- * Get a new random bookie, but ensure that it isn't one that is already
- * in the ensemble for the ledger.
- */
- private BookieSocketAddress getNewBookie(final List<BookieSocketAddress>
bookiesAlreadyInEnsemble,
- final List<BookieSocketAddress> availableBookies)
- throws BKException.BKNotEnoughBookiesException {
- ArrayList<BookieSocketAddress> candidates = new
ArrayList<BookieSocketAddress>();
- candidates.addAll(availableBookies);
- candidates.removeAll(bookiesAlreadyInEnsemble);
- if (candidates.size() == 0) {
- throw new BKException.BKNotEnoughBookiesException();
- }
- return candidates.get(rand.nextInt(candidates.size()));
- }
-
- /**
* This method asynchronously recovers a given ledger if any of the ledger
* entries were stored on the failed bookie.
*
- * @param bookieSrc
- * Source bookie that had a failure. We want to replicate the
+ * @param bookiesSrc
+ * Source bookies that had a failure. We want to replicate the
* ledger fragments that were stored there.
* @param lId
* Ledger id we want to recover.
- * @param ledgerIterCb
+ * @param dryrun
+ * printing the recovery plan without actually recovering
bookies
+ * @param skipOpenLedgers
+ * Skip recovering open ledgers.
+ * @param finalLedgerIterCb
* IterationCallback to invoke once we've recovered the current
* ledger.
- * @param availableBookies
- * List of Bookie Servers that are available to use for
- * replicating data on the failed bookie. This could contain a
- * single bookie server if the user explicitly chose a bookie
- * server to replicate data to.
*/
- private void recoverLedger(final BookieSocketAddress bookieSrc, final long
lId,
- final AsyncCallback.VoidCallback ledgerIterCb, final
List<BookieSocketAddress> availableBookies) {
+ private void recoverLedger(final Set<BookieSocketAddress> bookiesSrc,
final long lId, final boolean dryrun,
+ final boolean skipOpenLedgers, final
AsyncCallback.VoidCallback finalLedgerIterCb) {
if (LOG.isDebugEnabled()) {
LOG.debug("Recovering ledger : {}", lId);
}
asyncOpenLedgerNoRecovery(lId, new OpenCallback() {
@Override
public void openComplete(int rc, final LedgerHandle lh, Object
ctx) {
- if (rc != Code.OK.intValue()) {
+ if (rc != BKException.Code.OK) {
LOG.error("BK error opening ledger: " + lId,
BKException.create(rc));
- ledgerIterCb.processResult(rc, null, null);
+ finalLedgerIterCb.processResult(rc, null, null);
return;
}
LedgerMetadata lm = lh.getLedgerMetadata();
+ if (skipOpenLedgers && !lm.isClosed() && !lm.isInRecovery()) {
+ LOG.info("Skip recovering open ledger {}.", lId);
+ try {
+ lh.close();
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ } catch (BKException bke) {
+ LOG.warn("Error on closing ledger handle for {}.",
lId);
+ }
+ finalLedgerIterCb.processResult(BKException.Code.OK, null,
null);
+ return;
+ }
+
+ boolean fenceRequired = false;
Review comment:
still isn't clear to me. fenceRequired is only set when 1) a ledger is not
closed 2) last ensemble contain the src bookie. are you suggesting me pushing
both 1) and 2) into containBookies function? this doesn't sound correct to me
though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
