Skip to content

Commit 13e67f9

Browse files
ethqunzhonglhotari
ethqunzhong
authored and
lhotari
committed
remove in address2Region while bookie left to get correct rack info (#4504)
(cherry picked from commit 07de650)
1 parent 26626a9 commit 13e67f9

File tree

2 files changed

+136
-0
lines changed

2 files changed

+136
-0
lines changed

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,23 @@ protected String parseBookieRegion(BookieId addr) {
117117

118118
@Override
119119
public void handleBookiesThatLeft(Set<BookieId> leftBookies) {
120+
// case 1: (In some situation, eg, Broker and bookie restart concurrently.)
121+
//1. Bookie X join cluster for the first time, encounters a region exception, and `address2Region` record X's
122+
// region as default-region.
123+
//2. Bookie X left cluster and is removed from knownBookies, but address2Region retains the information of
124+
// bookie X.
125+
//3. update Bookie X's rack info, and calling `onBookieRackChange` will only update address2Region for
126+
// addresses present in knownBookies; therefore, bookie X's region info is not updated.
127+
//4. Bookie X join cluster again, since address2Region contains the previous default-region information,
128+
// getRegion will directly use cached data, resulting of an incorrect region.
129+
130+
// The bookie region is initialized to "default-region" in address2Region.
131+
// We should ensure that when a bookie leaves the cluster,
132+
// we also clean up the corresponding region information for that bookie in address2Region,
133+
// so that it can update the correct region for the bookie during onBookieRackChange and
134+
// handleBookiesThatJoined.
135+
// to avoid traffic skew in ensemble selection.
136+
leftBookies.forEach(address2Region::remove);
120137
super.handleBookiesThatLeft(leftBookies);
121138

122139
for (TopologyAwareEnsemblePlacementPolicy policy: perRegionPlacement.values()) {

‎bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import static org.apache.bookkeeper.client.RegionAwareEnsemblePlacementPolicy.REPP_REGIONS_TO_WRITE;
2828
import static org.apache.bookkeeper.client.RoundRobinDistributionSchedule.writeSetFromValues;
2929
import static org.apache.bookkeeper.feature.SettableFeatureProvider.DISABLE_ALL;
30+
import static org.junit.jupiter.api.Assertions.assertNotNull;
31+
import static org.junit.jupiter.api.Assertions.assertNull;
3032
import static org.mockito.Mockito.mock;
3133
import static org.mockito.Mockito.times;
3234
import static org.mockito.Mockito.verify;
@@ -2046,4 +2048,121 @@ public void testNewEnsemblePickLocalRegionBookies()
20462048
LOG.info("Bookie1 Count: {}, Bookie8 Count: {}, Bookie9 Count: {}", bookie1Count, bookie8Count, bookie9Count);
20472049

20482050
}
2051+
2052+
@Test
2053+
public void testBookieLeftThenJoinWithDNSResolveFailed() throws Exception {
2054+
2055+
BookieSocketAddress addr1 = new BookieSocketAddress("127.0.1.1", 3181);
2056+
BookieSocketAddress addr2 = new BookieSocketAddress("127.0.1.2", 3181);
2057+
BookieSocketAddress addr3 = new BookieSocketAddress("127.0.1.3", 3181);
2058+
BookieSocketAddress addr4 = new BookieSocketAddress("127.0.1.4", 3181);
2059+
2060+
// init dns mapping
2061+
// 2. mock dns resolver failed, use default region and rack.
2062+
// addr1 rack info. /region-1/default-rack -> /default-region/default-rack.
2063+
2064+
// 1. mock addr1 dns resolver failed and use default region and rack.
2065+
StaticDNSResolver.addNodeToRack(addr1.getHostName(), "/default-region/default-rack");
2066+
StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/region-1/default-rack");
2067+
StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/region-2/default-rack");
2068+
StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/region-3/default-rack");
2069+
2070+
// init cluster
2071+
Set<BookieId> addrs = Sets.newHashSet(addr1.toBookieId(),
2072+
addr2.toBookieId(), addr3.toBookieId(), addr4.toBookieId());
2073+
repp.onClusterChanged(addrs, new HashSet<>());
2074+
2075+
assertEquals(4, repp.knownBookies.size());
2076+
assertEquals("/default-region/default-rack", repp.knownBookies.get(addr1.toBookieId()).getNetworkLocation());
2077+
assertEquals("/region-1/default-rack", repp.knownBookies.get(addr2.toBookieId()).getNetworkLocation());
2078+
assertEquals("/region-2/default-rack", repp.knownBookies.get(addr3.toBookieId()).getNetworkLocation());
2079+
assertEquals("/region-3/default-rack", repp.knownBookies.get(addr4.toBookieId()).getNetworkLocation());
2080+
2081+
assertEquals(4, repp.perRegionPlacement.size());
2082+
TopologyAwareEnsemblePlacementPolicy unknownRegionPlacement = repp.perRegionPlacement.get("UnknownRegion");
2083+
assertEquals(1, unknownRegionPlacement.knownBookies.keySet().size());
2084+
assertEquals("/default-region/default-rack",
2085+
unknownRegionPlacement.knownBookies.get(addr1.toBookieId()).getNetworkLocation());
2086+
2087+
TopologyAwareEnsemblePlacementPolicy region1Placement = repp.perRegionPlacement.get("region-1");
2088+
assertEquals(1, region1Placement.knownBookies.keySet().size());
2089+
assertEquals("/region-1/default-rack",
2090+
region1Placement.knownBookies.get(addr2.toBookieId()).getNetworkLocation());
2091+
2092+
TopologyAwareEnsemblePlacementPolicy region2Placement = repp.perRegionPlacement.get("region-2");
2093+
assertEquals(1, region2Placement.knownBookies.keySet().size());
2094+
assertEquals("/region-2/default-rack",
2095+
region2Placement.knownBookies.get(addr3.toBookieId()).getNetworkLocation());
2096+
2097+
TopologyAwareEnsemblePlacementPolicy region3Placement = repp.perRegionPlacement.get("region-3");
2098+
assertEquals(1, region3Placement.knownBookies.keySet().size());
2099+
assertEquals("/region-3/default-rack",
2100+
region3Placement.knownBookies.get(addr4.toBookieId()).getNetworkLocation());
2101+
2102+
assertEquals("UnknownRegion", repp.address2Region.get(addr1.toBookieId()));
2103+
assertEquals("region-1", repp.address2Region.get(addr2.toBookieId()));
2104+
assertEquals("region-2", repp.address2Region.get(addr3.toBookieId()));
2105+
assertEquals("region-3", repp.address2Region.get(addr4.toBookieId()));
2106+
2107+
// 2. addr1 bookie shutdown and decommission
2108+
addrs.remove(addr1.toBookieId());
2109+
repp.onClusterChanged(addrs, new HashSet<>());
2110+
2111+
assertEquals(3, repp.knownBookies.size());
2112+
assertNull(repp.knownBookies.get(addr1.toBookieId()));
2113+
assertEquals("/region-1/default-rack", repp.knownBookies.get(addr2.toBookieId()).getNetworkLocation());
2114+
assertEquals("/region-2/default-rack", repp.knownBookies.get(addr3.toBookieId()).getNetworkLocation());
2115+
assertEquals("/region-3/default-rack", repp.knownBookies.get(addr4.toBookieId()).getNetworkLocation());
2116+
2117+
// UnknownRegion,region-1,region-2,region-3
2118+
assertEquals(4, repp.perRegionPlacement.size());
2119+
// after addr1 bookie left, it should remove from locally address2Region
2120+
assertNull(repp.address2Region.get(addr1.toBookieId()));
2121+
assertEquals("region-1", repp.address2Region.get(addr2.toBookieId()));
2122+
assertEquals("region-2", repp.address2Region.get(addr3.toBookieId()));
2123+
assertEquals("region-3", repp.address2Region.get(addr4.toBookieId()));
2124+
2125+
2126+
// 3. addr1 bookie start and join
2127+
addrs.add(addr1.toBookieId());
2128+
repp.onClusterChanged(addrs, new HashSet<>());
2129+
2130+
assertEquals(4, repp.knownBookies.size());
2131+
assertEquals("/default-region/default-rack", repp.knownBookies.get(addr1.toBookieId()).getNetworkLocation());
2132+
assertEquals("/region-1/default-rack", repp.knownBookies.get(addr2.toBookieId()).getNetworkLocation());
2133+
assertEquals("/region-2/default-rack", repp.knownBookies.get(addr3.toBookieId()).getNetworkLocation());
2134+
assertEquals("/region-3/default-rack", repp.knownBookies.get(addr4.toBookieId()).getNetworkLocation());
2135+
2136+
// UnknownRegion,region-1,region-2,region-3
2137+
assertEquals(4, repp.perRegionPlacement.size());
2138+
assertEquals("UnknownRegion", repp.address2Region.get(addr1.toBookieId()));
2139+
// addr1 bookie belongs to unknown region
2140+
unknownRegionPlacement = repp.perRegionPlacement.get("UnknownRegion");
2141+
assertEquals(1, unknownRegionPlacement.knownBookies.keySet().size());
2142+
assertEquals("/default-region/default-rack",
2143+
unknownRegionPlacement.knownBookies.get(addr1.toBookieId()).getNetworkLocation());
2144+
2145+
// 4. Update the correct rack.
2146+
// change addr1 rack info. /default-region/default-rack -> /region-1/default-rack.
2147+
List<BookieSocketAddress> bookieAddressList = new ArrayList<>();
2148+
List<String> rackList = new ArrayList<>();
2149+
bookieAddressList.add(addr1);
2150+
rackList.add("/region-1/default-rack");
2151+
// onBookieRackChange
2152+
StaticDNSResolver.changeRack(bookieAddressList, rackList);
2153+
2154+
assertEquals(4, repp.perRegionPlacement.size());
2155+
// addr1 bookie, oldRegion=default-region, newRegion=region-1
2156+
assertEquals("region-1", repp.address2Region.get(addr1.toBookieId()));
2157+
2158+
unknownRegionPlacement = repp.perRegionPlacement.get("UnknownRegion");
2159+
assertEquals(0, unknownRegionPlacement.knownBookies.keySet().size());
2160+
assertNotNull(unknownRegionPlacement.historyBookies.get(addr1.toBookieId()));
2161+
2162+
2163+
region1Placement = repp.perRegionPlacement.get("region-1");
2164+
assertEquals(2, region1Placement.knownBookies.keySet().size());
2165+
assertEquals("/region-1/default-rack",
2166+
region1Placement.knownBookies.get(addr1.toBookieId()).getNetworkLocation());
2167+
}
20492168
}

0 commit comments

Comments
 (0)