Fix NetworkTopologyImpl#getLeaves returning set with null value in case of non existing scope
Descriptions of the changes in this PR:
NetworkTopologyImpl#getLeaves(String scope) was returning set with null value, when requested scope did not exist.
I've added null check in NetworkTopologyImpl#doGetLeaves(String scope), used by NetworkTopologyImpl#getLeaves method to not have null node returned as leaf node.
It would be nice to have this merged also with non master branch of bookeeper so maintenance releases could have that fix.
### Motivation
I've found my Apache Pulsar cluster polluted with log:
```
12:01:46.462 pulsar2-dev pulsar-broker {"logLevel":"ERROR","logThread":"pulsar-io-23-1","logger":"org.apache.bookkeeper.client.RackawareEnsemblePlacementPolicyImpl","message":"found non-BookieNode: null as leaf of defaultrack: /default-rack","stack_trace":null}
```
After digging I've found that, because https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java#L324
was returning HashSet with null node, when no bookkeepers were in /default-rack.
### Changes
- Modified doGetLeaves() method to return empty HashSet instead of HashSet with null element when scope node can't be found.
Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Andrey Yegorov <andrey.yegorov@datastax.com>
This closes #2632 from mkozioro/fix-do-get-leavesdiff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopologyImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopologyImpl.java
index 52b3325..123f13c 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopologyImpl.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopologyImpl.java
@@ -754,6 +754,10 @@
private Set<Node> doGetLeaves(String scope) {
Node node = getNode(scope);
Set<Node> leafNodes = new HashSet<Node>();
+ if (node == null) {
+ return leafNodes;
+ }
+
if (!(node instanceof InnerNode)) {
leafNodes.add(node);
} else {
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/net/NetworkTopologyImplTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/net/NetworkTopologyImplTest.java
new file mode 100644
index 0000000..a6cc8c3
--- /dev/null
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/net/NetworkTopologyImplTest.java
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.bookkeeper.net;
+
+import static org.junit.Assert.assertTrue;
+
+import java.util.Set;
+import org.junit.Test;
+
+/**
+ * Tests for {@link NetworkTopologyImpl}.
+ */
+public class NetworkTopologyImplTest {
+
+ @Test
+ public void getLeavesShouldReturnEmptySetForNonExistingScope() {
+ NetworkTopologyImpl networkTopology = new NetworkTopologyImpl();
+ final Set<Node> leaves = networkTopology.getLeaves("/non-existing-scope");
+ assertTrue(leaves.isEmpty());
+ }
+
+ @Test
+ public void getLeavesShouldReturnNodesInScope() {
+ // GIVEN
+ // Topology with two racks and 1 bookie in each rack.
+ NetworkTopologyImpl networkTopology = new NetworkTopologyImpl();
+
+ String rack0Scope = "/rack-0";
+ BookieId bookieIdScopeRack0 = BookieId.parse("bookieIdScopeRack0");
+ BookieNode bookieRack0ScopeNode = new BookieNode(bookieIdScopeRack0, rack0Scope);
+
+ String rack1Scope = "/rack-1";
+ BookieId bookieIdScopeRack1 = BookieId.parse("bookieIdScopeRack1");
+ BookieNode bookieRack1ScopeNode = new BookieNode(bookieIdScopeRack1, rack1Scope);
+
+ networkTopology.add(bookieRack0ScopeNode);
+ networkTopology.add(bookieRack1ScopeNode);
+
+ // WHEN
+ Set<Node> leavesScopeRack0 = networkTopology.getLeaves(rack0Scope);
+ Set<Node> leavesScopeRack1 = networkTopology.getLeaves(rack1Scope);
+
+ // THEN
+ assertTrue(leavesScopeRack0.size() == 1);
+ assertTrue(leavesScopeRack0.contains(bookieRack0ScopeNode));
+
+ assertTrue(leavesScopeRack1.size() == 1);
+ assertTrue(leavesScopeRack1.contains(bookieRack1ScopeNode));
+ }
+
+ @Test
+ public void getLeavesShouldReturnLeavesThatAreNotInExcludedScope() {
+ // GIVEN
+ // Topology with three racks and 1 bookie in each rack.
+ NetworkTopologyImpl networkTopology = new NetworkTopologyImpl();
+
+ String rack0Scope = "/rack-0";
+ BookieId bookieIdScopeRack0 = BookieId.parse("bookieIdScopeRack0");
+ BookieNode bookieRack0ScopeNode = new BookieNode(bookieIdScopeRack0, rack0Scope);
+
+ String rack1Scope = "/rack-1";
+ BookieId bookieIdScopeRack1 = BookieId.parse("bookieIdScopeRack1");
+ BookieNode bookieRack1ScopeNode = new BookieNode(bookieIdScopeRack1, rack1Scope);
+
+ String rack2Scope = "/rack-2";
+ BookieId bookieIdScopeRack2 = BookieId.parse("bookieIdScopeRack2");
+ BookieNode bookieRack2ScopeNode = new BookieNode(bookieIdScopeRack2, rack2Scope);
+
+ networkTopology.add(bookieRack0ScopeNode);
+ networkTopology.add(bookieRack1ScopeNode);
+ networkTopology.add(bookieRack2ScopeNode);
+
+ // Excluded scopes are beginned with '~' character.
+ String scopeExcludingRack1 = "~/rack-1";
+
+ // WHEN
+ // ask for leaves not being at rack1 scope.
+ Set<Node> leavesExcludingRack2Scope = networkTopology.getLeaves(scopeExcludingRack1);
+
+ // THEN
+ assertTrue(leavesExcludingRack2Scope.size() == 2);
+ assertTrue(leavesExcludingRack2Scope.contains(bookieRack0ScopeNode));
+ assertTrue(leavesExcludingRack2Scope.contains(bookieRack2ScopeNode));
+ }
+}
\ No newline at end of file
