Skip to content

Fix no known bookies after reset racks for all BKs #4128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zymap merged 2 commits into from
Nov 20, 2023
Merged

Fix no known bookies after reset racks for all BKs #4128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91cf3f0
Fix no known bookies after reset racks for all BKs
poorbarcode Nov 9, 2023
5349936
address comment
poorbarcode Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopologyImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,9 @@ public void remove(Node node) {
if (rack == null) {
numOfRacks--;
}
if (clusterMap.numOfLeaves == 0) {
depthOfAllLeaves = -1;
}
}
if (LOG.isDebugEnabled()) {
LOG.debug("NetworkTopology became:\n" + this);
Expand Down
40 changes: 40 additions & 0 deletions bookkeeper-server/src/test/java/org/apache/bookkeeper/net/NetworkTopologyImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,46 @@ public void getLeavesShouldReturnNodesInScope() {
assertTrue(leavesScopeRack1.contains(bookieRack1ScopeNode));
}

@Test
public void testRestartBKWithNewRackDepth() {
NetworkTopologyImpl networkTopology = new NetworkTopologyImpl();
String dp1Rack = "/rack-1";
String dp2Rack = "/dp/rack-1";
BookieId bkId1 = BookieId.parse("bookieIdScopeRack0");
BookieId bkId2 = BookieId.parse("bookieIdScopeRack1");

// Register 2 BKs with depth 1 rack.
BookieNode dp1BkNode1 = new BookieNode(bkId1, dp1Rack);
BookieNode dp1BkNode2 = new BookieNode(bkId2, dp1Rack);
networkTopology.add(dp1BkNode1);

@horizonzy horizonzy Nov 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a case for the middle state will be better.
The only one bks shutdown, then restart it with depth 2 rack, it also can't add successfully.

@poorbarcode poorbarcode Nov 10, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

networkTopology.add(dp1BkNode2);

// Update one BK with depth 2 rack.
// Assert it can not be added due to different depth.
networkTopology.remove(dp1BkNode1);
BookieNode dp2BkNode1 = new BookieNode(bkId1, dp2Rack);
try {
networkTopology.add(dp2BkNode1);
fail("Expected add node failed caused by different depth of rack");
} catch (NetworkTopologyImpl.InvalidTopologyException ex) {
// Expected ex.
}
Set<Node> leaves = networkTopology.getLeaves(dp1Rack);
assertEquals(leaves.size(), 1);
assertTrue(leaves.contains(dp1BkNode2));

// Update all Bks with depth 2 rack.
// Verify update success.
networkTopology.remove(dp1BkNode2);
BookieNode dp2BkNode2 = new BookieNode(bkId2, dp2Rack);
networkTopology.add(dp2BkNode1);
networkTopology.add(dp2BkNode2);
leaves = networkTopology.getLeaves(dp2Rack);
assertEquals(leaves.size(), 2);
assertTrue(leaves.contains(dp2BkNode1));
assertTrue(leaves.contains(dp2BkNode2));
}

@Test
public void getLeavesShouldReturnLeavesThatAreNotInExcludedScope() {
// GIVEN
Expand Down