Fix disk weight ensemble infinite loop bug

[hangc0276]

Motivation

In the RackawareEnsemblePlacementPolicyImpl#selectRandomInternal method, if bookieNode in the ensemble list is not added to the excludeBookies set and the diskWeight feature is enabled, this method will fall into an infinite loop.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java

Lines 773 to 777 in 3ed93a0

	protected List<BookieNode> selectRandomInternal(List<BookieNode> bookiesToSelectFrom,
	int numBookies,
	Set<Node> excludeBookies,
	Predicate<BookieNode> predicate,
	Ensemble<BookieNode> ensemble)

The root cause is the newly selected bookies already exist in the ensemble list, leading the ensemble.addNode(bookie) method to return false and the while loop continues.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java

Line 841 in 3ed93a0

if (ensemble.addNode(bookie)) {

In the next round of the loop, bookiesSeenSoFar.size() will never equal to bookiesToSelectFrom.size() due to the wRSelection set doesn't exclude the bookies in the ensemble set.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java

Lines 815 to 822 in 3ed93a0

	if (bookiesSeenSoFar.size() == bookiesToSelectFrom.size()) {
	// If we have gone through the whole available list of bookies,
	// and yet haven't been able to satisfy the ensemble request, bail out.
	// We don't want to loop infinitely.
	break;
	}
	bookie = wRSelection.getNextRandom();
	bookiesSeenSoFar.add(bookie);

Changes

Remove the following code to make sure the items in bookiesToSelectFrom are all included in wRSelection

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java

Lines 795 to 797 in 3ed93a0

	if (excludeBookies.contains(n)) {
	continue;
	}

(Describe: what changes you have made)

Master Issue: #

---

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.

---

[wenbingshen]

Good catch!
Leave a minor comment:

[shoothzj]

I think keeping input param excludeBookies and remove this if clause is a little werid and confusing. cc @wenbingshen @dlg99 @eolivelli

[horizonzy]

Nice catch.

[horizonzy]

We would better reset StaticDNSResolver at the test end.

[hangc0276]

In the setUp method, it will reset the StaticDNSResolver. So we do not need to reset it in the end of the test.

[wenbingshen]

LGTM

[eolivelli]

+1