This is an automated email from the ASF dual-hosted git repository. sijie pushed a commit to branch branch-4.5 in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit e789023c1a86507212559652f60fba49a8800f34 Author: Andrey Yegorov <[email protected]> AuthorDate: Tue Aug 15 18:41:49 2017 -0700 BOOKKEEPER-1105: RackAwarePolicy: Failure to map node into rack may result in failure to add other nodes. - RackAwarePolicy's no longer uses /default-region if rack mapping fails unless required (by RegionAwarePolicy) - it no longer fails to add rest of nodes after one node's failed addition, - added unit tests - added counters for successful/failed bookie adds/removal (PR description content here)... UpdateLedgerOpTest failed but it seems to be known/unrelated issue. Author: Andrey Yegorov <[email protected]> Reviewers: Sijie Guo <[email protected]> This closes #425 from dlg99/fix/rackaware --- .../bookkeeper/bookie/BookKeeperServerStats.java | 2 + .../client/RackawareEnsemblePlacementPolicy.java | 1 + .../RackawareEnsemblePlacementPolicyImpl.java | 185 ++++++++++++++++----- .../client/RegionAwareEnsemblePlacementPolicy.java | 12 +- .../bookkeeper/net/CachedDNSToSwitchMapping.java | 6 + .../apache/bookkeeper/net/DNSToSwitchMapping.java | 13 +- .../java/org/apache/bookkeeper/net/NetUtils.java | 23 ++- .../org/apache/bookkeeper/net/NetworkTopology.java | 3 +- .../apache/bookkeeper/net/NetworkTopologyImpl.java | 3 +- .../apache/bookkeeper/net/ScriptBasedMapping.java | 5 +- .../TestRackawareEnsemblePlacementPolicy.java | 73 ++++---- ...ackawareEnsemblePlacementPolicyUsingScript.java | 78 ++++++++- .../TestRegionAwareEnsemblePlacementPolicy.java | 53 +++--- .../apache/bookkeeper/util/StaticDNSResolver.java | 5 +- .../src/test/resources/networkmappingscript.sh | 6 +- 15 files changed, 346 insertions(+), 122 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java index c589205..cf539ed 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java @@ -119,6 +119,8 @@ public interface BookKeeperServerStats { String SKIP_LIST_THROTTLING = "SKIP_LIST_THROTTLING"; String READ_LAST_ENTRY_NOENTRY_ERROR = "READ_LAST_ENTRY_NOENTRY_ERROR"; String LEDGER_CACHE_NUM_EVICTED_LEDGERS = "LEDGER_CACHE_NUM_EVICTED_LEDGERS"; + String BOOKIES_JOINED = "BOOKIES_JOINED"; + String BOOKIES_LEFT = "BOOKIES_LEFT"; // Gauge String NUM_INDEX_PAGES = "NUM_INDEX_PAGES"; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java index 0eb8b92..8126b96 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; + import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.net.DNSToSwitchMapping; import org.apache.bookkeeper.net.Node; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java index 020a101..e37f296 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java @@ -30,21 +30,16 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.locks.ReentrantReadWriteLock; +import com.google.common.base.Preconditions; +import org.apache.bookkeeper.bookie.BookKeeperServerStats; import org.apache.bookkeeper.client.BKException.BKNotEnoughBookiesException; import org.apache.bookkeeper.client.BookieInfoReader.BookieInfo; import org.apache.bookkeeper.client.WeightedRandomSelection.WeightedObject; import org.apache.bookkeeper.conf.ClientConfiguration; import org.apache.bookkeeper.conf.Configurable; import org.apache.bookkeeper.feature.FeatureProvider; -import org.apache.bookkeeper.net.BookieSocketAddress; -import org.apache.bookkeeper.net.DNSToSwitchMapping; -import org.apache.bookkeeper.net.NetUtils; -import org.apache.bookkeeper.net.NetworkTopology; -import org.apache.bookkeeper.net.NetworkTopologyImpl; -import org.apache.bookkeeper.net.Node; -import org.apache.bookkeeper.net.NodeBase; -import org.apache.bookkeeper.net.ScriptBasedMapping; -import org.apache.bookkeeper.net.StabilizeNetworkTopology; +import org.apache.bookkeeper.net.*; +import org.apache.bookkeeper.stats.OpStatsLogger; import org.apache.bookkeeper.stats.StatsLogger; import org.apache.bookkeeper.util.ReflectionUtils; import org.apache.commons.collections.CollectionUtils; @@ -56,6 +51,7 @@ import com.google.common.collect.Sets; import io.netty.util.HashedWheelTimer; import java.util.Optional; +import java.util.function.Supplier; /** * Simple rackware ensemble placement policy. @@ -77,11 +73,25 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen static class DefaultResolver implements DNSToSwitchMapping { + final Supplier<String> defaultRackSupplier; + + // for backwards compat + public DefaultResolver() { + this(() -> NetworkTopology.DEFAULT_REGION_AND_RACK); + } + + public DefaultResolver(Supplier<String> defaultRackSupplier) { + Preconditions.checkNotNull(defaultRackSupplier, "defaultRackSupplier should not be null"); + this.defaultRackSupplier = defaultRackSupplier; + } + @Override public List<String> resolve(List<String> names) { List<String> rNames = new ArrayList<String>(names.size()); for (@SuppressWarnings("unused") String name : names) { - rNames.add(NetworkTopology.DEFAULT_RACK); + final String defaultRack = defaultRackSupplier.get(); + Preconditions.checkNotNull(defaultRack, "defaultRack cannot be null"); + rNames.add(defaultRack); } return rNames; } @@ -93,6 +103,62 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen } + /** + * Decorator for any existing dsn resolver. + * Backfills returned data with appropriate default rack info. + */ + static class DNSResolverDecorator implements DNSToSwitchMapping { + + final Supplier<String> defaultRackSupplier; + final DNSToSwitchMapping resolver; + + DNSResolverDecorator(DNSToSwitchMapping resolver, Supplier<String> defaultRackSupplier) { + Preconditions.checkNotNull(resolver, "Resolver cannot be null"); + Preconditions.checkNotNull(defaultRackSupplier, "defaultRackSupplier should not be null"); + this.defaultRackSupplier = defaultRackSupplier; + this.resolver= resolver; + } + + public List<String> resolve(List<String> names) { + if (names == null) { + return Collections.emptyList(); + } + final String defaultRack = defaultRackSupplier.get(); + Preconditions.checkNotNull(defaultRack, "Default rack cannot be null"); + + List<String> rNames = resolver.resolve(names); + if (rNames != null && rNames.size() == names.size()) { + for (int i = 0; i < rNames.size(); ++i) { + if (rNames.get(i) == null) { + LOG.warn("Failed to resolve network location for {}, using default rack for it : {}.", + rNames.get(i), defaultRack); + rNames.set(i, defaultRack); + } + } + return rNames; + } + + LOG.warn("Failed to resolve network location for {}, using default rack for them : {}.", names, + defaultRack); + rNames = new ArrayList<>(names.size()); + + for (int i = 0; i < names.size(); ++i) { + rNames.add(defaultRack); + } + return rNames; + } + + @Override + public boolean useHostName() { + return resolver.useHostName(); + } + + @Override + public void reloadCachedMappings() { + resolver.reloadCachedMappings(); + } + } + // for now, we just maintain the writable bookies' topology protected NetworkTopology topology; protected DNSToSwitchMapping dnsResolver; @@ -104,7 +170,13 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen protected boolean reorderReadsRandom = false; protected boolean enforceDurability = false; protected int stabilizePeriodSeconds = 0; + // looks like these only assigned in the same thread as constructor, immediately after constructor; + // no need to make volatile protected StatsLogger statsLogger = null; + protected OpStatsLogger bookiesJoinedCounter = null; + protected OpStatsLogger bookiesLeftCounter = null; + + private String defaultRack = NetworkTopology.DEFAULT_RACK; RackawareEnsemblePlacementPolicyImpl() { this(false); @@ -135,10 +207,13 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen boolean isWeighted, int maxWeightMultiple, StatsLogger statsLogger) { + Preconditions.checkNotNull(statsLogger, "statsLogger should not be null, use NullStatsLogger instead."); this.statsLogger = statsLogger; + this.bookiesJoinedCounter = statsLogger.getOpStatsLogger(BookKeeperServerStats.BOOKIES_JOINED); + this.bookiesLeftCounter = statsLogger.getOpStatsLogger(BookKeeperServerStats.BOOKIES_LEFT); this.reorderReadsRandom = reorderReadsRandom; this.stabilizePeriodSeconds = stabilizePeriodSeconds; - this.dnsResolver = dnsResolver; + this.dnsResolver = new DNSResolverDecorator(dnsResolver, () -> this.getDefaultRack()); this.timer = timer; // create the network topology @@ -170,6 +245,22 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen } return this; } + + /* + * sets default rack for the policy. + * i.e. region-aware policy may want to have /region/rack while regular + * rack-aware policy needs /rack only since we cannot mix both styles + */ + public RackawareEnsemblePlacementPolicyImpl withDefaultRack(String rack) { + Preconditions.checkNotNull(rack, "Default rack cannot be null"); + + this.defaultRack = rack; + return this; + } + + public String getDefaultRack() { + return defaultRack; + } @Override public RackawareEnsemblePlacementPolicyImpl initialize(ClientConfiguration conf, @@ -189,7 +280,7 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen } } catch (RuntimeException re) { LOG.info("Failed to initialize DNS Resolver {}, used default subnet resolver.", dnsResolverName, re); - dnsResolver = new DefaultResolver(); + dnsResolver = new DefaultResolver(() -> this.getDefaultRack()); } } return initialize( @@ -224,11 +315,8 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen joinedBookies = Sets.difference(writableBookies, oldBookieSet).immutableCopy(); // dead bookies. deadBookies = Sets.difference(leftBookies, readOnlyBookies).immutableCopy(); - if (LOG.isDebugEnabled()) { - LOG.debug( - "Cluster changed : left bookies are {}, joined bookies are {}, while dead bookies are {}.", - new Object[] { leftBookies, joinedBookies, deadBookies }); - } + LOG.debug("Cluster changed : left bookies are {}, joined bookies are {}, while dead bookies are {}.", + leftBookies, joinedBookies, deadBookies); handleBookiesThatLeft(leftBookies); handleBookiesThatJoined(joinedBookies); if (this.isWeighted && (leftBookies.size() > 0 || joinedBookies.size() > 0)) { @@ -247,15 +335,27 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen @Override public void handleBookiesThatLeft(Set<BookieSocketAddress> leftBookies) { for (BookieSocketAddress addr : leftBookies) { - BookieNode node = knownBookies.remove(addr); - if(null != node) { - topology.remove(node); - if (this.isWeighted) { - this.bookieInfoMap.remove(node); + try { + BookieNode node = knownBookies.remove(addr); + if(null != node) { + topology.remove(node); + if (this.isWeighted) { + this.bookieInfoMap.remove(node); + } + + bookiesLeftCounter.registerSuccessfulValue(1L); + + if (LOG.isDebugEnabled()) { + LOG.debug("Cluster changed : bookie {} left from cluster.", addr); + } } - if (LOG.isDebugEnabled()) { - LOG.debug("Cluster changed : bookie {} left from cluster.", addr); + } catch (Throwable t) { + LOG.error("Unexpected exception while handling leaving bookie {}", addr, t); + if (bookiesLeftCounter != null ) { + bookiesLeftCounter.registerFailedValue(1L); } + // no need to re-throw; we want to process the rest of the bookies + // exception anyways will be caught/logged/suppressed in the ZK's event handler } } } @@ -264,14 +364,26 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen public void handleBookiesThatJoined(Set<BookieSocketAddress> joinedBookies) { // node joined for (BookieSocketAddress addr : joinedBookies) { - BookieNode node = createBookieNode(addr); - topology.add(node); - knownBookies.put(addr, node); - if (this.isWeighted) { - this.bookieInfoMap.putIfAbsent(node, new BookieInfo()); - } - if (LOG.isDebugEnabled()) { - LOG.debug("Cluster changed : bookie {} joined the cluster.", addr); + try { + BookieNode node = createBookieNode(addr); + topology.add(node); + knownBookies.put(addr, node); + if (this.isWeighted) { + this.bookieInfoMap.putIfAbsent(node, new BookieInfo()); + } + + bookiesJoinedCounter.registerSuccessfulValue(1L); + + if (LOG.isDebugEnabled()) { + LOG.debug("Cluster changed : bookie {} joined the cluster.", addr); + } + } catch (Throwable t) { + // topology.add() throws unchecked exception + LOG.error("Unexpected exception while handling joining bookie {}", addr, t); + + bookiesJoinedCounter.registerFailedValue(1L); + // no need to re-throw; we want to process the rest of the bookies + // exception anyways will be caught/logged/suppressed in the ZK's event handler } } } @@ -361,7 +473,7 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen String curRack; if (null == prevNode) { if ((null == localNode) || - localNode.getNetworkLocation().equals(NetworkTopology.DEFAULT_RACK)) { + defaultRack.equals(localNode.getNetworkLocation())) { curRack = NodeBase.ROOT; } else { curRack = localNode.getNetworkLocation(); @@ -650,10 +762,9 @@ class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsemblePlacemen if (numBookies == 0) { return newBookies; } - if (LOG.isDebugEnabled()) { - LOG.debug("Failed to find {} bookies : excludeBookies {}, allBookies {}.", new Object[] { - numBookies, excludeBookies, bookiesToSelectFrom }); - } + LOG.warn("Failed to find {} bookies : excludeBookies {}, allBookies {}.", + numBookies, excludeBookies, bookiesToSelectFrom); + throw new BKNotEnoughBookiesException(); } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java index 89415a1..7d4d160 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java @@ -34,6 +34,7 @@ import org.apache.bookkeeper.feature.Feature; import org.apache.bookkeeper.feature.FeatureProvider; import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.net.DNSToSwitchMapping; +import org.apache.bookkeeper.net.NetUtils; import org.apache.bookkeeper.net.NetworkTopology; import org.apache.bookkeeper.net.Node; import org.apache.bookkeeper.net.NodeBase; @@ -84,7 +85,7 @@ public class RegionAwareEnsemblePlacementPolicy extends RackawareEnsemblePlaceme String region = address2Region.get(addr); if (null == region) { String networkLocation = resolveNetworkLocation(addr); - if (NetworkTopology.DEFAULT_RACK.equals(networkLocation)) { + if (NetworkTopology.DEFAULT_REGION_AND_RACK.equals(networkLocation)) { region = UNKNOWN_REGION; } else { String[] parts = networkLocation.split(NodeBase.PATH_SEPARATOR_STR); @@ -128,7 +129,8 @@ public class RegionAwareEnsemblePlacementPolicy extends RackawareEnsemblePlaceme if (null == perRegionPlacement.get(region)) { perRegionPlacement.put(region, new RackawareEnsemblePlacementPolicy() .initialize(dnsResolver, timer, this.reorderReadsRandom, this.stabilizePeriodSeconds, - this.isWeighted, this.maxWeightMultiple, statsLogger)); + this.isWeighted, this.maxWeightMultiple, statsLogger) + .withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK)); } Set<BookieSocketAddress> regionSet = perRegionClusterChange.get(region); @@ -160,7 +162,8 @@ public class RegionAwareEnsemblePlacementPolicy extends RackawareEnsemblePlaceme HashedWheelTimer timer, FeatureProvider featureProvider, StatsLogger statsLogger) { - super.initialize(conf, optionalDnsResolver, timer, featureProvider, statsLogger); + super.initialize(conf, optionalDnsResolver, timer, featureProvider, statsLogger) + .withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); myRegion = getLocalRegion(localNode); enableValidation = conf.getBoolean(REPP_ENABLE_VALIDATION, true); // We have to statically provide regions we want the writes to go through and how many regions @@ -174,7 +177,8 @@ public class RegionAwareEnsemblePlacementPolicy extends RackawareEnsemblePlaceme for (String region: regions) { perRegionPlacement.put(region, new RackawareEnsemblePlacementPolicy(true) .initialize(dnsResolver, timer, this.reorderReadsRandom, this.stabilizePeriodSeconds, - this.isWeighted, this.maxWeightMultiple, statsLogger)); + this.isWeighted, this.maxWeightMultiple, statsLogger) + .withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK)); } minRegionsForDurability = conf.getInt(REPP_MINIMUM_REGIONS_FOR_DURABILITY, MINIMUM_REGIONS_FOR_DURABILITY_DEFAULT); if (minRegionsForDurability > 0) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/CachedDNSToSwitchMapping.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/CachedDNSToSwitchMapping.java index d7ff251..8176831 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/CachedDNSToSwitchMapping.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/CachedDNSToSwitchMapping.java @@ -46,6 +46,12 @@ public class CachedDNSToSwitchMapping extends AbstractDNSToSwitchMapping { this.rawMapping = rawMapping; } + // we'll use IP Address for these mappings. + @Override + public boolean useHostName() { + return false; + } + /** * @param names a list of hostnames to probe for being cached * @return the hosts from 'names' that have not been cached previously diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/DNSToSwitchMapping.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/DNSToSwitchMapping.java index 6156993..96df655 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/DNSToSwitchMapping.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/DNSToSwitchMapping.java @@ -41,7 +41,7 @@ public interface DNSToSwitchMapping { * <p/> * * If a name cannot be resolved to a rack, the implementation - * should return {@link NetworkTopology#DEFAULT_RACK}. This + * should return {@link NetworkTopology#DEFAULT_REGION_AND_RACK}. This * is what the bundled implementations do, though it is not a formal requirement * * @param names the list of hosts to resolve (can be empty) @@ -57,4 +57,15 @@ public interface DNSToSwitchMapping { * will get a chance to see the new data. */ public void reloadCachedMappings(); + + /** + * Hints what to use with implementation when InetSocketAddress is converted + * to String: + * hostname (addr.getHostName(), default) + * or IP address (addr.getAddress().getHostAddress()) + * @return true if hostname, false if IP address. Default is true. + */ + default boolean useHostName() { + return true; + } } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetUtils.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetUtils.java index d954d04..6172e73 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetUtils.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetUtils.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,22 +65,20 @@ public class NetUtils { public static String resolveNetworkLocation(DNSToSwitchMapping dnsResolver, InetSocketAddress addr) { List<String> names = new ArrayList<String>(1); - if (dnsResolver instanceof CachedDNSToSwitchMapping) { - names.add(addr.getAddress().getHostAddress()); - } else { + + if (dnsResolver.useHostName()) { names.add(addr.getHostName()); } + else { + names.add(addr.getAddress().getHostAddress()); + } + // resolve network addresses List<String> rNames = dnsResolver.resolve(names); - String netLoc; - if (null == rNames) { - logger.warn("Failed to resolve network location for {}, using default rack for them : {}.", names, - NetworkTopology.DEFAULT_RACK); - netLoc = NetworkTopology.DEFAULT_RACK; - } else { - netLoc = rNames.get(0); - } - return netLoc; + Preconditions.checkNotNull(rNames, "DNS Resolver should not return null response."); + Preconditions.checkState(rNames.size() == 1, "Expected exactly one element"); + + return rNames.get(0); } } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopology.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopology.java index 18f3ec9..073be16 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopology.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetworkTopology.java @@ -25,7 +25,8 @@ import java.util.Set; public interface NetworkTopology { public final static String DEFAULT_REGION = "/default-region"; - public final static String DEFAULT_RACK = "/default-region/default-rack"; + public final static String DEFAULT_RACK = "/default-rack"; + public final static String DEFAULT_REGION_AND_RACK = DEFAULT_REGION + DEFAULT_RACK; /** * Add a node to the network topology diff --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 78c4fe4..59e6349 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 @@ -379,12 +379,13 @@ public class NetworkTopologyImpl implements NetworkTopology { netlock.writeLock().lock(); try { if ((depthOfAllLeaves != -1) && (depthOfAllLeaves != newDepth)) { - LOG.error("Error: can't add leaf node at depth " + newDepth + " to topology:\n" + oldTopoStr); + LOG.error("Error: can't add leaf node {} at depth {} to topology:\n{}", node, newDepth, oldTopoStr); throw new InvalidTopologyException("Invalid network topology. " + "You cannot have a rack and a non-rack node at the same level of the network topology."); } Node rack = getNodeForNetworkLocation(node); if (rack != null && !(rack instanceof InnerNode)) { + LOG.error("Unexpected data node {} at an illegal network location", node); throw new IllegalArgumentException("Unexpected data node " + node.toString() + " at an illegal network location"); } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/ScriptBasedMapping.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/ScriptBasedMapping.java index dd8563c..d9cd000 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/ScriptBasedMapping.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/net/ScriptBasedMapping.java @@ -163,10 +163,7 @@ public final class ScriptBasedMapping extends CachedDNSToSwitchMapping { } if (scriptName == null) { - for (int i = 0; i < names.size(); i++) { - m.add(NetworkTopology.DEFAULT_RACK); - } - return m; + return null; } String output = runResolveCommand(names); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java index 7ed90fd..ad3fa55 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java @@ -40,6 +40,8 @@ import org.apache.bookkeeper.conf.ClientConfiguration; import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.net.DNSToSwitchMapping; import org.apache.bookkeeper.net.NetworkTopology; +import org.apache.bookkeeper.stats.NullStatsLogger; +import org.apache.bookkeeper.stats.StatsLogger; import org.apache.bookkeeper.util.StaticDNSResolver; import org.junit.Test; import org.slf4j.Logger; @@ -60,9 +62,9 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { protected void setUp() throws Exception { super.setUp(); StaticDNSResolver.reset(); - StaticDNSResolver.addNodeToRack(InetAddress.getLocalHost().getHostAddress(), NetworkTopology.DEFAULT_RACK); - StaticDNSResolver.addNodeToRack("127.0.0.1", NetworkTopology.DEFAULT_RACK); - StaticDNSResolver.addNodeToRack("localhost", NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(InetAddress.getLocalHost().getHostAddress(), NetworkTopology.DEFAULT_REGION_AND_RACK); + StaticDNSResolver.addNodeToRack("127.0.0.1", NetworkTopology.DEFAULT_REGION_AND_RACK); + StaticDNSResolver.addNodeToRack("localhost", NetworkTopology.DEFAULT_REGION_AND_RACK); LOG.info("Set up static DNS Resolver."); conf.setProperty(REPP_DNS_RESOLVER_CLASS, StaticDNSResolver.class.getName()); addr1 = new BookieSocketAddress("127.0.0.2", 3181); @@ -71,8 +73,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION + "/rack1"); - StaticDNSResolver.addNodeToRack(addr2.getHostName(), NetworkTopology.DEFAULT_RACK); - StaticDNSResolver.addNodeToRack(addr3.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr2.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); + StaticDNSResolver.addNodeToRack(addr3.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr4.getHostName(), NetworkTopology.DEFAULT_REGION + "/rack2"); ensemble.add(addr1); ensemble.add(addr2); @@ -88,7 +90,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { conf.getTimeoutTimerNumTicks()); repp = new RackawareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); } @Override @@ -107,10 +110,11 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { @Test(timeout = 60000) public void testNodeDown() throws Exception { repp.uninitalize(); - updateMyRack(NetworkTopology.DEFAULT_RACK); + updateMyRack(NetworkTopology.DEFAULT_REGION_AND_RACK); repp = new RackawareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); addrs.add(addr1); @@ -138,7 +142,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r1/rack1"); repp = new RackawareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); // Update cluster Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); @@ -169,7 +174,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r1/rack1"); repp = new RackawareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); // Update cluster Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); @@ -199,7 +205,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r1/rack1"); repp = new RackawareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); // Update cluster Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); @@ -230,7 +237,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/default-region/r3"); @@ -253,7 +260,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/default-region/r3"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/default-region/r4"); @@ -280,7 +287,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/default-region/r3"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/default-region/r4"); @@ -334,7 +341,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.3", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.4", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/default-region/r2"); @@ -368,11 +375,11 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr7 = new BookieSocketAddress("127.0.0.8", 3181); BookieSocketAddress addr8 = new BookieSocketAddress("127.0.0.9", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/default-region/r3"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/default-region/r4"); - StaticDNSResolver.addNodeToRack(addr5.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr5.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr6.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr7.getHostName(), "/default-region/r3"); StaticDNSResolver.addNodeToRack(addr8.getHostName(), "/default-region/r4"); @@ -407,7 +414,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/default-region/r2"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/default-region/r3"); @@ -430,7 +437,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.4", 3181); // update dns mapping StaticDNSResolver.addNodeToRack(addr1.getSocketAddress().getAddress().getHostAddress(), - NetworkTopology.DEFAULT_RACK); + NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); StaticDNSResolver.addNodeToRack(addr3.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); StaticDNSResolver.addNodeToRack(addr4.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); @@ -444,7 +451,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { int multiple = 10; conf.setDiskWeightBasedPlacementEnabled(true); conf.setBookieMaxWeightMultipleForWeightBasedPlacement(-1); // no max cap on weight - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); repp.onClusterChanged(addrs, new HashSet<BookieSocketAddress>()); Map<BookieSocketAddress, BookieInfo> bookieInfoMap = new HashMap<BookieSocketAddress, BookieInfo>(); @@ -477,7 +485,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.4", 3181); // update dns mapping StaticDNSResolver.reset(); - StaticDNSResolver.addNodeToRack(addr1.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); StaticDNSResolver.addNodeToRack(addr3.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r3"); StaticDNSResolver.addNodeToRack(addr4.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r4"); @@ -491,7 +499,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { int multiple = 10, maxMultiple = 4; conf.setDiskWeightBasedPlacementEnabled(true); conf.setBookieMaxWeightMultipleForWeightBasedPlacement(maxMultiple); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); repp.onClusterChanged(addrs, new HashSet<BookieSocketAddress>()); Map<BookieSocketAddress, BookieInfo> bookieInfoMap = new HashMap<BookieSocketAddress, BookieInfo>(); @@ -542,7 +551,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr9 = new BookieSocketAddress("127.0.0.9", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); StaticDNSResolver.addNodeToRack(addr3.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); StaticDNSResolver.addNodeToRack(addr4.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); @@ -567,7 +576,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { int maxMultiple = 4; conf.setDiskWeightBasedPlacementEnabled(true); conf.setBookieMaxWeightMultipleForWeightBasedPlacement(maxMultiple); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); repp.onClusterChanged(addrs, new HashSet<BookieSocketAddress>()); Map<BookieSocketAddress, BookieInfo> bookieInfoMap = new HashMap<BookieSocketAddress, BookieInfo>(); @@ -621,7 +631,7 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr5 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); StaticDNSResolver.addNodeToRack(addr3.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r2"); StaticDNSResolver.addNodeToRack(addr4.getSocketAddress().getAddress().getHostAddress(), NetworkTopology.DEFAULT_REGION + "/r3"); @@ -637,7 +647,8 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { int maxMultiple = 4; conf.setDiskWeightBasedPlacementEnabled(true); conf.setBookieMaxWeightMultipleForWeightBasedPlacement(maxMultiple); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); repp.onClusterChanged(addrs, new HashSet<BookieSocketAddress>()); Map<BookieSocketAddress, BookieInfo> bookieInfoMap = new HashMap<BookieSocketAddress, BookieInfo>(); @@ -687,10 +698,11 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { @Test(timeout = 60000) public void testNodeWithFailures() throws Exception { repp.uninitalize(); - updateMyRack(NetworkTopology.DEFAULT_RACK); + updateMyRack(NetworkTopology.DEFAULT_REGION_AND_RACK); repp = new RackawareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); addrs.add(addr1); @@ -715,13 +727,14 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { @Test(timeout = 60000) public void testPlacementOnStabilizeNetworkTopology() throws Exception { repp.uninitalize(); - updateMyRack(NetworkTopology.DEFAULT_RACK); + updateMyRack(NetworkTopology.DEFAULT_REGION_AND_RACK); repp = new RackawareEnsemblePlacementPolicy(); ClientConfiguration confLocal = new ClientConfiguration(); confLocal.addConfiguration(conf); confLocal.setNetworkTopologyStabilizePeriodSeconds(99999); - repp.initialize(confLocal, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(confLocal, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); + repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK); Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); addrs.add(addr1); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicyUsingScript.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicyUsingScript.java index 387b83a..82f9291 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicyUsingScript.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicyUsingScript.java @@ -38,6 +38,7 @@ import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.net.CommonConfigurationKeys; import org.apache.bookkeeper.net.DNSToSwitchMapping; import org.apache.bookkeeper.net.ScriptBasedMapping; +import org.apache.bookkeeper.stats.NullStatsLogger; import org.apache.bookkeeper.util.Shell; import org.junit.After; import org.junit.Assume; @@ -83,7 +84,7 @@ public class TestRackawareEnsemblePlacementPolicyUsingScript { conf.getTimeoutTimerNumTicks()); repp = new RackawareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); } @After @@ -167,6 +168,81 @@ public class TestRackawareEnsemblePlacementPolicyUsingScript { } } + /* + * Test that even in case of script mapping error + * we are getting default rack that makes sense for the policy. + * i.e. if all nodes in rack-aware policy use /rack format + * but one gets node /default-region/default-rack the node addition to topology will fail. + * + * This case adds node with non-default rack, then adds nodes with one on default rack. + */ + @Test(timeout = 60000) + public void testReplaceBookieWithScriptMappingError() throws Exception { + ignoreTestIfItIsWindowsOS(); + BookieSocketAddress addr0 = new BookieSocketAddress("127.0.0.0", 3181); // error mapping to rack here + BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.1", 3181); // /1 rack + BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.2", 3181); // /2 rack + + // Update cluster, add node that maps to non-default rack + Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); + addrs.add(addr1); + + repp.onClusterChanged(addrs, new HashSet<BookieSocketAddress>()); + + addrs = new HashSet<BookieSocketAddress>(); + addrs.add(addr0); + addrs.add(addr1); + addrs.add(addr2); + repp.onClusterChanged(addrs, new HashSet<BookieSocketAddress>()); + + // replace node under r2 + Set<BookieSocketAddress> excludedAddrs = new HashSet<BookieSocketAddress>(); + excludedAddrs.add(addr1); + BookieSocketAddress replacedBookie = repp.replaceBookie(1, 1, 1, null, new HashSet<BookieSocketAddress>(), addr2, excludedAddrs); + + assertFalse(addr1.equals(replacedBookie)); + assertFalse(addr2.equals(replacedBookie)); + assertTrue(addr0.equals(replacedBookie)); + } + + /* + * Test that even in case of script mapping error + * we are getting default rack that makes sense for the policy. + * i.e. if all nodes in rack-aware policy use /rack format + * but one gets node /default-region/default-rack the node addition to topology will fail. + * + * This case adds node with default rack, then adds nodes with non-default rack. + * Almost the same as testReplaceBookieWithScriptMappingError but different order of addition. + */ + @Test(timeout = 60000) + public void testReplaceBookieWithScriptMappingError2() throws Exception { + ignoreTestIfItIsWindowsOS(); + BookieSocketAddress addr0 = new BookieSocketAddress("127.0.0.0", 3181); // error mapping to rack here + BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.1", 3181); // /1 rack + BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.2", 3181); // /2 rack + + // Update cluster, add node that maps to default rack first + Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); + addrs.add(addr0); + + repp.onClusterChanged(addrs, new HashSet<BookieSocketAddress>()); + + addrs = new HashSet<BookieSocketAddress>(); + addrs.add(addr0); + addrs.add(addr1); + addrs.add(addr2); + repp.onClusterChanged(addrs, new HashSet<BookieSocketAddress>()); + + // replace node under r2 + Set<BookieSocketAddress> excludedAddrs = new HashSet<BookieSocketAddress>(); + excludedAddrs.add(addr1); + BookieSocketAddress replacedBookie = repp.replaceBookie(1, 1, 1, null, new HashSet<BookieSocketAddress>(), addr2, excludedAddrs); + + assertFalse(addr1.equals(replacedBookie)); + assertFalse(addr2.equals(replacedBookie)); + assertTrue(addr0.equals(replacedBookie)); + } + @Test(timeout = 60000) public void testNewEnsembleWithSingleRack() throws Exception { ignoreTestIfItIsWindowsOS(); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java index 74cf1f5..fb45537 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java @@ -38,6 +38,7 @@ import org.apache.bookkeeper.feature.SettableFeatureProvider; import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.net.DNSToSwitchMapping; import org.apache.bookkeeper.net.NetworkTopology; +import org.apache.bookkeeper.stats.NullStatsLogger; import org.apache.bookkeeper.util.BookKeeperConstants; import org.apache.bookkeeper.util.StaticDNSResolver; import org.junit.Test; @@ -71,7 +72,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { protected void setUp() throws Exception { super.setUp(); StaticDNSResolver.reset(); - updateMyRack(NetworkTopology.DEFAULT_RACK); + updateMyRack(NetworkTopology.DEFAULT_REGION_AND_RACK); LOG.info("Set up static DNS Resolver."); conf.setProperty(REPP_DNS_RESOLVER_CLASS, StaticDNSResolver.class.getName()); @@ -81,8 +82,8 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping StaticDNSResolver.addNodeToRack(addr1.getHostName(), "/r1/rack1"); - StaticDNSResolver.addNodeToRack(addr2.getHostName(), NetworkTopology.DEFAULT_RACK); - StaticDNSResolver.addNodeToRack(addr3.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr2.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); + StaticDNSResolver.addNodeToRack(addr3.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/r1/rack2"); ensemble.add(addr1); ensemble.add(addr2); @@ -98,7 +99,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { conf.getTimeoutTimerNumTicks()); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); } @Override @@ -110,10 +111,10 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { @Test(timeout = 60000) public void testNotReorderReadIfInDefaultRack() throws Exception { repp.uninitalize(); - updateMyRack(NetworkTopology.DEFAULT_RACK); + updateMyRack(NetworkTopology.DEFAULT_REGION_AND_RACK); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); List<Integer> reorderSet = repp.reorderReadSequence(ensemble, writeSet, new HashMap<BookieSocketAddress, Long>()); assertFalse(reorderSet == writeSet); @@ -126,7 +127,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r1/rack3"); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); addrs.add(addr1); @@ -152,7 +153,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r2/rack1"); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); List<Integer> reoderSet = repp.reorderReadSequence(ensemble, writeSet, new HashMap<BookieSocketAddress, Long>()); LOG.info("reorder set : {}", reoderSet); @@ -166,7 +167,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r1/rack1"); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); // Update cluster Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); @@ -195,7 +196,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r1/rack1"); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); // Update cluster Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); @@ -226,7 +227,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r1/rack1"); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); // Update cluster Set<BookieSocketAddress> addrs = new HashSet<BookieSocketAddress>(); @@ -257,7 +258,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/region1/r1"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/region1/r2"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/default-region/r3"); @@ -280,7 +281,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/region1/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/region2/r3"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/region3/r4"); @@ -307,7 +308,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/region2/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/region3/r3"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/region4/r4"); @@ -334,7 +335,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/region2/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/region3/r3"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/region4/r4"); @@ -362,7 +363,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { public void testNewEnsembleWithSingleRegion() throws Exception { repp.uninitalize(); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.3", 3181); BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); @@ -393,13 +394,13 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { public void testNewEnsembleWithMultipleRegions() throws Exception { repp.uninitalize(); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.3", 3181); BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); // update dns mapping - StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_RACK); + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/region1/r2"); StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/region1/r2"); StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/region1/r2"); @@ -471,7 +472,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { public void testNewEnsembleWithThreeRegions() throws Exception { repp.uninitalize(); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.3", 3181); BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); @@ -538,7 +539,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { repp.uninitalize(); repp = new RegionAwareEnsemblePlacementPolicy(); conf.setProperty(REPP_DISALLOW_BOOKIE_PLACEMENT_IN_REGION_FEATURE_NAME, "disallowBookies"); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, featureProvider, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, featureProvider, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.3", 3181); BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); @@ -617,7 +618,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { repp = new RegionAwareEnsemblePlacementPolicy(); conf.setProperty(REPP_REGIONS_TO_WRITE, "region1;region2;region3;region4;region5"); conf.setProperty(REPP_MINIMUM_REGIONS_FOR_DURABILITY, 5); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.1.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.1.0.3", 3181); BookieSocketAddress addr3 = new BookieSocketAddress("127.1.0.4", 3181); @@ -722,7 +723,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { } conf.setProperty(REPP_DISALLOW_BOOKIE_PLACEMENT_IN_REGION_FEATURE_NAME, "disallowBookies"); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, featureProvider, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, featureProvider, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.1.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.1.0.3", 3181); BookieSocketAddress addr3 = new BookieSocketAddress("127.1.0.4", 3181); @@ -859,7 +860,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { conf.setProperty(REPP_ENABLE_DURABILITY_ENFORCEMENT_IN_REPLACE, true); } - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, featureProvider, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, featureProvider, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.1.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.1.0.3", 3181); BookieSocketAddress addr3 = new BookieSocketAddress("127.1.0.4", 3181); @@ -924,7 +925,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { conf.setProperty(REPP_REGIONS_TO_WRITE, "region1;region2;region3;region4;region5"); conf.setProperty(REPP_MINIMUM_REGIONS_FOR_DURABILITY, 5); conf.setProperty(REPP_ENABLE_VALIDATION, false); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.3", 3181); BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); @@ -977,7 +978,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { updateMyRack("/" + myRegion); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181); BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.3", 3181); @@ -1207,7 +1208,7 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { updateMyRack("/r2/rack1"); repp = new RegionAwareEnsemblePlacementPolicy(); - repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, null); + repp.initialize(conf, Optional.<DNSToSwitchMapping>empty(), timer, DISABLE_ALL, NullStatsLogger.INSTANCE); BookieSocketAddress addr5 = new BookieSocketAddress("127.0.0.6", 3181); BookieSocketAddress addr6 = new BookieSocketAddress("127.0.0.7", 3181); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/util/StaticDNSResolver.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/util/StaticDNSResolver.java index b02fdce..b08586c 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/util/StaticDNSResolver.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/util/StaticDNSResolver.java @@ -48,7 +48,7 @@ public class StaticDNSResolver extends AbstractDNSToSwitchMapping { public static String getRack(String name) { String rack = name2Racks.get(name); if (null == rack) { - rack = NetworkTopology.DEFAULT_RACK; + rack = NetworkTopology.DEFAULT_REGION_AND_RACK; } return rack; } @@ -71,9 +71,6 @@ public class StaticDNSResolver extends AbstractDNSToSwitchMapping { List<String> racks = new ArrayList<String>(); for (String n : names) { String rack = name2Racks.get(n); - if (null == rack) { - rack = NetworkTopology.DEFAULT_RACK; - } if (LOG.isDebugEnabled()) { LOG.debug("Resolve name {} to rack {}.", n, rack); } diff --git a/bookkeeper-server/src/test/resources/networkmappingscript.sh b/bookkeeper-server/src/test/resources/networkmappingscript.sh index 73ba5db..9eed3f6 100755 --- a/bookkeeper-server/src/test/resources/networkmappingscript.sh +++ b/bookkeeper-server/src/test/resources/networkmappingscript.sh @@ -27,9 +27,13 @@ # 127.0.0.2 - /2 # 199.12.34.21 - /1 # This script file is used just for testing purpose +# rack 0 returns script error (non-zero error code) for var in "$@" do i=$((${#var}-1)) + if [ "${var:$i:1}" == "0" ]; then + exit 1 + fi echo /${var:$i:1} -done +done \ No newline at end of file -- To stop receiving notification emails like this one, please contact "[email protected]" <[email protected]>.
