asafm commented on code in PR #3779:
URL: https://github.com/apache/bookkeeper/pull/3779#discussion_r1105629056
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java:
##########
@@ -675,6 +675,8 @@ protected BookieNode selectRandomFromRack(String netPath,
Set<Node> excludeBooki
}
return bn;
}
+ LOG.warn("Failed to select bookie node from path: {}, leaves: {},
exclude Bookies: {}, ensemble: {}",
Review Comment:
I wouldn't log that deep.
1st caller to this method is:
```java
// first attempt to select one from local rack
try {
return selectRandomFromRack(networkLoc, excludeBookies,
predicate, ensemble);
} catch (BKNotEnoughBookiesException e) {
/*
* there is no enough bookie from local rack, select bookies from
* the whole cluster and exclude the racks specified at
* <tt>excludeRacks</tt>.
*/
return selectFromNetworkLocation(excludeRacks, excludeBookies,
predicate, ensemble, fallbackToRandom);
}
```
For this caller, it's not a WARN yet, since we may still choose a bookie
another rack which satisfies rack awareness.
We can print INFO in the catch here.
2nd caller:
```java
// select one from local rack
try {
return selectRandomFromRack(networkLoc, excludeBookies,
predicate, ensemble);
} catch (BKNotEnoughBookiesException e) {
if (!fallbackToRandom) {
LOG.error(
"Failed to choose a bookie from {} : "
+ "excluded {},
enforceMinNumRacksPerWriteQuorum is enabled so giving up.",
networkLoc, excludeBookies);
throw e;
}
LOG.warn("Failed to choose a bookie from {} : "
+ "excluded {}, fallback to choose bookie randomly from
the cluster.",
networkLoc, excludeBookies);
// randomly choose one from whole cluster, ignore the provided
predicate.
return selectRandom(1, excludeBookies, predicate,
ensemble).get(0);
```
Here we have the error or WARN anyway, so no need to add any log here.
Since there are so many ways to this method to throw an exception, I would
add a reason message at each throw, and include it in the logs printed which
receive it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
