diff --git a/eureka-client/src/main/java/com/netflix/discovery/AbstractAzToRegionMapper.java b/eureka-client/src/main/java/com/netflix/discovery/AbstractAzToRegionMapper.java index 49c8e19e79..977ff23af6 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/AbstractAzToRegionMapper.java +++ b/eureka-client/src/main/java/com/netflix/discovery/AbstractAzToRegionMapper.java @@ -40,6 +40,7 @@ public List get() { }); private final Map availabilityZoneVsRegion = new ConcurrentHashMap(); + private final Map parsedAzCache = new ConcurrentHashMap(); private String[] regionsToFetch; protected AbstractAzToRegionMapper(EurekaClientConfig clientConfig) { @@ -53,6 +54,7 @@ public synchronized void setRegionsToFetch(String[] regionsToFetch) { this.regionsToFetch = regionsToFetch; logger.info("Fetching availability zone to region mapping for regions {}", (Object) regionsToFetch); availabilityZoneVsRegion.clear(); + parsedAzCache.clear(); for (String remoteRegion : regionsToFetch) { Set availabilityZones = getZonesForARegion(remoteRegion); if (null == availabilityZones @@ -83,6 +85,7 @@ public synchronized void setRegionsToFetch(String[] regionsToFetch) { } else { logger.info("Regions to fetch is null. Erasing older mapping if any."); availabilityZoneVsRegion.clear(); + parsedAzCache.clear(); this.regionsToFetch = EMPTY_STR_ARRAY; } } @@ -96,9 +99,15 @@ public synchronized void setRegionsToFetch(String[] regionsToFetch) { @Override public String getRegionForAvailabilityZone(String availabilityZone) { - String region = availabilityZoneVsRegion.get(availabilityZone); + String region = parsedAzCache.get(availabilityZone); if (null == region) { - return parseAzToGetRegion(availabilityZone); + region = availabilityZoneVsRegion.get(availabilityZone); + if (null == region) { + region = parseAzToGetRegion(availabilityZone); + } + if (region != null) { + parsedAzCache.put(availabilityZone, region); + } } return region; } diff --git a/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java b/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java index e04a778b1a..53b930e27e 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java +++ b/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java @@ -1001,7 +1001,7 @@ private boolean fetchRegistry(boolean forceFullRegistryFetch) { || (!Strings.isNullOrEmpty(clientConfig.getRegistryRefreshSingleVipAddress())) || forceFullRegistryFetch || (applications == null) - || (applications.getRegisteredApplications().size() == 0) + || applications.isRegisteredApplicationsEmpty() || (applications.getVersion() == -1)) //Client application does not have latest library supporting delta { logger.info("Disable delta property : {}", clientConfig.shouldDisableDelta()); @@ -1009,7 +1009,7 @@ private boolean fetchRegistry(boolean forceFullRegistryFetch) { logger.info("Force full registry fetch : {}", forceFullRegistryFetch); logger.info("Application is null : {}", (applications == null)); logger.info("Registered Applications size is zero : {}", - (applications.getRegisteredApplications().size() == 0)); + applications.isRegisteredApplicationsEmpty()); logger.info("Application version is -1: {}", (applications.getVersion() == -1)); getAndStoreFullRegistry(); } else { diff --git a/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java b/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java index c3c21039f2..39bec5904d 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java +++ b/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java @@ -424,6 +424,15 @@ protected void autoMarshalEligible(Object o, JsonGenerator jgen) { public static class InstanceInfoDeserializer extends JsonDeserializer { private static char[] BUF_AT_CLASS = "@class".toCharArray(); + /** Extract uppercase from current JsonParser cursor. Avoids lambda capture. */ + private static String toUpperCase(JsonParser jp) { + try { + return jp.getText().toUpperCase(); + } catch (IOException e) { + throw new RuntimeJsonMappingException(e.getMessage()); + } + } + enum InstanceInfoField { HOSTNAME(ELEM_HOST), INSTANCE_ID(ELEM_INSTANCE_ID), @@ -515,14 +524,7 @@ public InstanceInfo deserialize(JsonParser jp, DeserializationContext context) t break; case APP: builder.setAppNameForDeser( - intern.apply(jp, CacheScope.APPLICATION_SCOPE, - ()->{ - try { - return jp.getText().toUpperCase(); - } catch (IOException e) { - throw new RuntimeJsonMappingException(e.getMessage()); - } - })); + intern.apply(jp, CacheScope.APPLICATION_SCOPE, InstanceInfoDeserializer::toUpperCase)); break; case IP: builder.setIPAddr(intern.apply(jp)); @@ -590,14 +592,8 @@ public InstanceInfo deserialize(JsonParser jp, DeserializationContext context) t builder.setHealthCheckUrlsForDeser(null, intern.apply(jp.getText())); break; case APPGROUPNAME: - builder.setAppGroupNameForDeser(intern.apply(jp, CacheScope.GLOBAL_SCOPE, - ()->{ - try { - return jp.getText().toUpperCase(); - } catch (IOException e) { - throw new RuntimeJsonMappingException(e.getMessage()); - } - })); + builder.setAppGroupNameForDeser( + intern.apply(jp, CacheScope.GLOBAL_SCOPE, InstanceInfoDeserializer::toUpperCase)); break; case HOMEPAGEURL: builder.setHomePageUrlForDeser(intern.apply(jp.getText())); @@ -638,7 +634,9 @@ public InstanceInfo deserialize(JsonParser jp, DeserializationContext context) t String key = intern.apply(jp, CacheScope.GLOBAL_SCOPE); jsonToken = jp.nextToken(); String value = intern.apply(jp, CacheScope.APPLICATION_SCOPE ); - metadataMap = Optional.ofNullable(metadataMap).orElseGet(METADATA_MAP_SUPPLIER); + if (metadataMap == null) { + metadataMap = METADATA_MAP_SUPPLIER.get(); + } metadataMap.put(key, value); } }; diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java index e069298ca5..020476ed09 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java @@ -28,6 +28,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; @@ -135,7 +136,11 @@ public void removeInstance(InstanceInfo i) { */ @JsonProperty("instance") public List getInstances() { - return Optional.ofNullable(shuffledInstances.get()).orElseGet(this::getInstancesAsIsFromEureka); + List instances = shuffledInstances.get(); + if (instances == null) { + instances = this.getInstancesAsIsFromEureka(); + } + return instances; } /** @@ -152,6 +157,18 @@ public List getInstancesAsIsFromEureka() { } } + /** + * Iterate over instances without creating a defensive copy. + * Package-private to avoid exposing unsynchronized iteration to external callers. + * Callers should be sure that this is a quick iteration. + */ + void forEachInstance(Consumer consumer) { + synchronized (instances) { + for (InstanceInfo info : instances) { + consumer.accept(info); + } + } + } /** * Get the instance info that matches the given id. diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index 80dbb88a1b..24a232dfa4 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -20,7 +20,6 @@ import java.util.AbstractQueue; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -31,9 +30,11 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.stream.Collectors; +import com.netflix.discovery.util.MapUtil; + import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; @@ -65,17 +66,53 @@ @JsonRootName("applications") public class Applications { private static class VipIndexSupport { - final AbstractQueue instances = new ConcurrentLinkedQueue<>(); + // Progressive list: emptyList (0) -> singletonList (1) -> ArrayList (2+) + // This avoids CLQ and Node allocations. 56% of VIPs have exactly 1 instance. + private List instances = Collections.emptyList(); final AtomicLong roundRobinIndex = new AtomicLong(0); - final AtomicReference> vipList = new AtomicReference<>(Collections.emptyList()); + private volatile List vipList = Collections.emptyList(); + + void addInstance(InstanceInfo info) { + int size = instances.size(); + if (size == 0) { + // 0 -> 1: use singletonList (56% of VIPs stop here) + instances = Collections.singletonList(info); + } else if (size == 1) { + // 1 -> 2: transition singletonList to ArrayList. + // Capacity 12 chosen based on prod data analysis: covers 81% of multi-instance + // VIPs without resize (spikes at 6, 9, 12 instances from 3-AZ deployments). + // ArrayList grows 1.5x (12->18->27), aligning well with common sizes. + // Capacity 12 minimizes total allocation vs smaller capacities. + InstanceInfo first = instances.get(0); + ArrayList list = new ArrayList<>(12); + list.add(first); + list.add(info); + instances = list; + } else { + // 2+ -> n: append to ArrayList + ((ArrayList) instances).add(info); + } + } + + int instanceCount() { + return instances.size(); + } + + List getInstances() { + return instances; + } public AtomicLong getRoundRobinIndex() { return roundRobinIndex; } - public AtomicReference> getVipList() { + List getVipList() { return vipList; } + + void setVipList(List vipList) { + this.vipList = vipList; + } } private static final String STATUS_DELIMITER = "_"; @@ -137,6 +174,17 @@ public List getRegisteredApplications() { return new ArrayList(this.applications); } + /** + * Returns whether there are any registered applications. + * This is more efficient than {@code getRegisteredApplications().isEmpty()} + * as it avoids creating a defensive copy. + * + * @return true if there are no registered applications + */ + public boolean isRegisteredApplicationsEmpty() { + return this.applications.isEmpty(); + } + /** * Gets the registered application for the given * application name. @@ -161,8 +209,7 @@ public Application getRegisteredApplications(String appName) { public List getInstancesByVirtualHostName(String virtualHostName) { return Optional.ofNullable(this.virtualHostNameAppMap.get(virtualHostName.toUpperCase(Locale.ROOT))) .map(VipIndexSupport::getVipList) - .map(AtomicReference::get) - .orElseGet(Collections::emptyList); + .orElseGet(Collections::emptyList); } /** @@ -177,8 +224,7 @@ public List getInstancesByVirtualHostName(String virtualHostName) public List getInstancesBySecureVirtualHostName(String secureVirtualHostName) { return Optional.ofNullable(this.secureVirtualHostNameAppMap.get(secureVirtualHostName.toUpperCase(Locale.ROOT))) .map(VipIndexSupport::getVipList) - .map(AtomicReference::get) - .orElseGet(Collections::emptyList); + .orElseGet(Collections::emptyList); } /** @@ -244,11 +290,19 @@ public String getReconcileHashCode() { * the map to populate */ public void populateInstanceCountMap(Map instanceCountMap) { - for (Application app : this.getRegisteredApplications()) { - for (InstanceInfo info : app.getInstancesAsIsFromEureka()) { - AtomicInteger instanceCount = instanceCountMap.computeIfAbsent(info.getStatus().name(), - k -> new AtomicInteger(0)); - instanceCount.incrementAndGet(); + // accrue here as lightweight as possible + int[] statusCounts = new int[InstanceStatus.values().length]; + Consumer countByStatus = info -> statusCounts[info.getStatus().ordinal()]++; + for (Application app : this.applications) { + app.forEachInstance(countByStatus); + } + + // now convert it over to the API form in a single pass + for (InstanceStatus status : InstanceStatus.values()) { + int count = statusCounts[status.ordinal()]; + if (count > 0) { + instanceCountMap.computeIfAbsent(status.name(), k -> new AtomicInteger(0)) + .addAndGet(count); } } } @@ -305,8 +359,8 @@ private void shuffleInstances(boolean filterUpInstances, @Nullable Map remoteRegionsRegistry, @Nullable EurekaClientConfig clientConfig, @Nullable InstanceRegionChecker instanceRegionChecker) { - Map secureVirtualHostNameAppMap = new HashMap<>(); - Map virtualHostNameAppMap = new HashMap<>(); + Map secureVirtualHostNameAppMap = MapUtil.newHashMapWithExpectedSize(this.secureVirtualHostNameAppMap.size()); + Map virtualHostNameAppMap = MapUtil.newHashMapWithExpectedSize(this.virtualHostNameAppMap.size()); for (Application application : appNameApplicationMap.values()) { if (indexByRemoteRegions) { application.shuffleAndStoreInstances(remoteRegionsRegistry, clientConfig, instanceRegionChecker); @@ -348,21 +402,72 @@ public AtomicLong getNextIndex(String virtualHostname, boolean secure) { * */ private void shuffleAndFilterInstances(Map srcMap, boolean filterUpInstances) { - Random shuffleRandom = new Random(); for (Map.Entry entries : srcMap.entrySet()) { - VipIndexSupport vipIndexSupport = entries.getValue(); - AbstractQueue vipInstances = vipIndexSupport.instances; - final List filteredInstances; - if (filterUpInstances) { - filteredInstances = vipInstances.stream().filter(ii -> ii.getStatus() == InstanceStatus.UP) - .collect(Collectors.toCollection(() -> new ArrayList<>(vipInstances.size()))); - } else { - filteredInstances = new ArrayList(vipInstances); + shuffleAndFilterInstances(entries.getValue(), filterUpInstances, shuffleRandom); + } + } + + /** + * Shuffle and filter instances for a single VIP. + */ + private void shuffleAndFilterInstances(VipIndexSupport vipIndexSupport, boolean filterUpInstances, Random shuffleRandom) { + List instances = vipIndexSupport.getInstances(); + int size = instances.size(); + + // Empty: nothing to do + if (size == 0) { + vipIndexSupport.setVipList(instances); + return; + } + + // Single instance: no shuffle needed, check status if filtering + if (size == 1) { + InstanceInfo instance = instances.get(0); + boolean keep = !filterUpInstances || instance.getStatus() == InstanceStatus.UP; + vipIndexSupport.setVipList(keep ? instances : Collections.emptyList()); + return; + } + + // Multiple instances (2+): instances is always an ArrayList at this point + ArrayList list = (ArrayList) instances; + + // Filter in place if needed (no-op when all instances are UP) + if (filterUpInstances) { + filterToUpInstancesInPlace(list); + if (list.isEmpty()) { + vipIndexSupport.setVipList(Collections.emptyList()); + return; } - Collections.shuffle(filteredInstances, shuffleRandom); - vipIndexSupport.vipList.set(filteredInstances); - vipIndexSupport.roundRobinIndex.set(0); + } + + // Shuffle in place and reuse + Collections.shuffle(list, shuffleRandom); + vipIndexSupport.setVipList(list); + } + + /** + * Filter list in place to keep only UP instances. Allocation-free. + */ + private static void filterToUpInstancesInPlace(ArrayList list) { + int size = list.size(); + int writeIndex = 0; + // shift forward all of the UP instances + for (int i = 0; i < size; i++) { + InstanceInfo instance = list.get(i); + if (instance.getStatus() == InstanceStatus.UP) { + if (writeIndex != i) { + list.set(writeIndex, instance); + } + writeIndex++; + } + } + // Truncate: remove tail elements. Allows old objects to be GCd. + // Array is not shrunk back, but, in the majority case this is not useful. + // More important that we clear the entries so the InstanceInfo elements + // can be released. + if (writeIndex < size) { + list.subList(writeIndex, size).clear(); } } @@ -370,16 +475,47 @@ private void shuffleAndFilterInstances(Map srcMap, bool * Add the instance to the given map based if the vip address matches with * that of the instance. Note that an instance can be mapped to multiple vip * addresses. - * */ private void addInstanceToMap(InstanceInfo info, String vipAddresses, Map vipMap) { - if (vipAddresses != null) { - String[] vipAddressArray = vipAddresses.toUpperCase(Locale.ROOT).split(","); - for (String vipAddress : vipAddressArray) { - VipIndexSupport vis = vipMap.computeIfAbsent(vipAddress, k -> new VipIndexSupport()); - vis.instances.add(info); - } + // This code path is quite hot on allocations. We apply common-case optimizations to minimize allocations. + // Gathered statistics from a real cluster: + // | Metric | Test | Prod | + // |-----------------------|--------|---------| + // | Total entries | N | 2x N | + // | Single VIP (no comma) | 91.1% | 91.7% | + // | 2 VIPs | 6.9% | 5.9% | + // | 3+ VIPs | 0.4% | 0.7% | + // | Empty | 1.6% | 1.7% | + // | Max VIPs per entry | 7 | 13 | + // | Avg string length | 29.5 | 25.8 | + // | Max string length | 204 | 468 | + + // Note: empty vipAddresses is intentionally allowed for backwards compatibility. + // Legacy behavior: "" creates a mapping with empty string key. + if (vipAddresses == null) { + return; } + + String upper = vipAddresses.toUpperCase(Locale.ROOT); + + // Fast path (91.7% of cases) single VIP: no split() -> byte[], no substring() -> String + int commaIndex = upper.indexOf(','); + if (commaIndex == -1) { + vipMap.computeIfAbsent(upper, k -> new VipIndexSupport()).addInstance(info); + return; + } + + // Multiple VIPs: uppercase once, then parse without split() byte[] allocation + int start = 0; + do { + String vipAddress = upper.substring(start, commaIndex); + vipMap.computeIfAbsent(vipAddress, k -> new VipIndexSupport()).addInstance(info); + start = commaIndex + 1; + } while ((commaIndex = upper.indexOf(',', start)) != -1); + + // Last segment + String vipAddress = upper.substring(start); + vipMap.computeIfAbsent(vipAddress, k -> new VipIndexSupport()).addInstance(info); } /** diff --git a/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java b/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java index 013e8ddbbf..3b65648f07 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java +++ b/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java @@ -45,6 +45,10 @@ public enum CacheScope { private final Map applicationCache; private final int lengthLimit = LENGTH_LIMIT; + // Reusable lookup buffers to avoid allocation on cache hits (single-threaded usage) + private final MutableArrayCharBuffer parserLookupBuffer = new MutableArrayCharBuffer(); + private final MutableStringCharBuffer stringLookupBuffer = new MutableStringCharBuffer(); + /** * adds a new DeserializerStringCache to the passed-in ObjectReader * @@ -203,48 +207,22 @@ public String apply(final JsonParser jp, CacheScope cacheScope) throws IOExcepti * * @param jp * @param cacheScope + * @param transform optional transform function applied to the parser text on cache miss * @return a possibly interned String * @throws IOException */ - public String apply(final JsonParser jp, CacheScope cacheScope, Supplier source) throws IOException { - return apply(CharBuffer.wrap(jp, source), cacheScope); - } - - /** - * returns a String that may be interned at app-scope to reduce heap - * consumption - * - * @param charValue - * @return a possibly interned String - */ - public String apply(final CharBuffer charValue) { - return apply(charValue, CacheScope.APPLICATION_SCOPE); - } - - /** - * returns a object of type T that may be interned at the specified scope to - * reduce heap consumption - * - * @param charValue - * @param cacheScope - * @param trabsform - * @return a possibly interned instance of T - */ - public String apply(CharBuffer charValue, CacheScope cacheScope) { - int keyLength = charValue.length(); - if ((lengthLimit < 0 || keyLength <= lengthLimit)) { + public String apply(final JsonParser jp, CacheScope cacheScope, Function transform) throws IOException { + parserLookupBuffer.reset(jp, transform); + int keyLength = parserLookupBuffer.length(); + if (lengthLimit < 0 || keyLength <= lengthLimit) { Map cache = (cacheScope == CacheScope.GLOBAL_SCOPE) ? globalCache : applicationCache; - String value = cache.get(charValue); + String value = cache.get(parserLookupBuffer); if (value == null) { - value = charValue.consume((k, v) -> { - cache.put(k, v); - }); - } else { - // System.out.println("cache hit"); + value = parserLookupBuffer.consume(cache::put); } return value; } - return charValue.toString(); + return parserLookupBuffer.toString(); } /** @@ -269,11 +247,15 @@ public String apply(final String stringValue) { */ public String apply(final String stringValue, CacheScope cacheScope) { if (stringValue != null && (lengthLimit < 0 || stringValue.length() <= lengthLimit)) { - return (String) (cacheScope == CacheScope.GLOBAL_SCOPE ? globalCache : applicationCache) - .computeIfAbsent(CharBuffer.wrap(stringValue), s -> { - logger.trace(" (string) writing new interned value {} into {} cache scope", stringValue, cacheScope); - return stringValue; - }); + stringLookupBuffer.reset(stringValue); + Map cache = (cacheScope == CacheScope.GLOBAL_SCOPE) ? globalCache : applicationCache; + String value = cache.get(stringLookupBuffer); + if (value == null) { + logger.trace(" (string) writing new interned value {} into {} cache scope", stringValue, cacheScope); + cache.put(new CharBuffer.StringCharBuffer(stringValue), stringValue); + value = stringValue; + } + return value; } return stringValue; } @@ -285,18 +267,6 @@ public int size() { private interface CharBuffer { static final int DEFAULT_VARIANT = -1; - public static CharBuffer wrap(JsonParser source, Supplier stringSource) throws IOException { - return new ArrayCharBuffer(source, stringSource); - } - - public static CharBuffer wrap(JsonParser source) throws IOException { - return new ArrayCharBuffer(source); - } - - public static CharBuffer wrap(String source) { - return new StringCharBuffer(source); - } - String consume(BiConsumer valueConsumer); int length(); @@ -305,118 +275,20 @@ public static CharBuffer wrap(String source) { OfInt chars(); - static class ArrayCharBuffer implements CharBuffer { - private final char[] source; - private final int offset; - private final int length; - private final Supplier valueTransform; - private final int variant; - private final int hash; - - ArrayCharBuffer(JsonParser source) throws IOException { - this(source, null); - } - - ArrayCharBuffer(JsonParser source, Supplier valueTransform) throws IOException { - this.source = source.getTextCharacters(); - this.offset = source.getTextOffset(); - this.length = source.getTextLength(); - this.valueTransform = valueTransform; - this.variant = valueTransform == null ? DEFAULT_VARIANT : System.identityHashCode(valueTransform.getClass()); - this.hash = 31 * arrayHash(this.source, offset, length) + variant; - } - - @Override - public int length() { - return length; - } - - @Override - public int variant() { - return variant; - } - - @Override - public int hashCode() { - return hash; - } - - @Override - public boolean equals(Object other) { - if (other instanceof CharBuffer) { - CharBuffer otherBuffer = (CharBuffer) other; - if (otherBuffer.length() == length) { - if (otherBuffer.variant() == variant) { - OfInt otherText = otherBuffer.chars(); - for (int i = offset; i < length; i++) { - if (source[i] != otherText.nextInt()) { - return false; - } - } - return true; - } - } - } - return false; - } - - @Override - public OfInt chars() { - return new OfInt() { - int index = offset; - int limit = index + length; - - @Override - public boolean hasNext() { - return index < limit; - } - - @Override - public int nextInt() { - return source[index++]; - } - }; - } - - @Override - public String toString() { - return valueTransform == null ? new String(this.source, offset, length) : valueTransform.get(); - } - - @Override - public String consume(BiConsumer valueConsumer) { - String key = new String(this.source, offset, length); - String value = valueTransform == null ? key : valueTransform.get(); - valueConsumer.accept(new StringCharBuffer(key, variant), value); - return value; - } - - private static int arrayHash(char[] a, int offset, int length) { - if (a == null) - return 0; - int result = 0; - int limit = offset + length; - for (int i = offset; i < limit; i++) { - result = 31 * result + a[i]; - } - return result; - } - } - static class StringCharBuffer implements CharBuffer { private final String source; private final int variant; private final int hashCode; StringCharBuffer(String source) { - this(source, -1); + this(source, DEFAULT_VARIANT); } StringCharBuffer(String source, int variant) { this.source = source; this.variant = variant; this.hashCode = 31 * source.hashCode() + variant; - } + } @Override public int hashCode() { @@ -483,4 +355,179 @@ public String consume(BiConsumer valueConsumer) { } } + + /** + * Mutable version of ArrayCharBuffer for reusable lookups. + * Reset before each use to avoid allocations on cache hits. + */ + private static class MutableArrayCharBuffer implements CharBuffer { + private JsonParser jp; + private char[] source; + private int offset; + private int length; + private Function valueTransform; + private int variant; + private int hash; + + void reset(JsonParser jp, Function valueTransform) throws IOException { + this.jp = jp; + this.source = jp.getTextCharacters(); + this.offset = jp.getTextOffset(); + this.length = jp.getTextLength(); + this.valueTransform = valueTransform; + this.variant = valueTransform == null ? DEFAULT_VARIANT : System.identityHashCode(valueTransform.getClass()); + this.hash = 31 * arrayHash(this.source, offset, length) + variant; + } + + @Override + public int length() { + return length; + } + + @Override + public int variant() { + return variant; + } + + @Override + public int hashCode() { + return hash; + } + + @Override + public boolean equals(Object other) { + if (other instanceof CharBuffer) { + CharBuffer otherBuffer = (CharBuffer) other; + if (otherBuffer.length() == length && otherBuffer.variant() == variant) { + OfInt otherText = otherBuffer.chars(); + for (int i = offset; i < offset + length; i++) { + if (source[i] != otherText.nextInt()) { + return false; + } + } + return true; + } + } + return false; + } + + @Override + public OfInt chars() { + return new OfInt() { + int index = offset; + int limit = index + length; + + @Override + public boolean hasNext() { + return index < limit; + } + + @Override + public int nextInt() { + return source[index++]; + } + }; + } + + @Override + public String toString() { + return valueTransform == null ? new String(this.source, offset, length) : valueTransform.apply(jp); + } + + @Override + public String consume(BiConsumer valueConsumer) { + String key = new String(this.source, offset, length); + String value = valueTransform == null ? key : valueTransform.apply(jp); + valueConsumer.accept(new CharBuffer.StringCharBuffer(key, variant), value); + return value; + } + + private static int arrayHash(char[] a, int offset, int length) { + if (a == null) + return 0; + int result = 0; + int limit = offset + length; + for (int i = offset; i < limit; i++) { + result = 31 * result + a[i]; + } + return result; + } + } + + /** + * Mutable version of StringCharBuffer for reusable lookups. + * Reset before each use to avoid allocations on cache hits. + */ + private static class MutableStringCharBuffer implements CharBuffer { + private String source; + private int hashCode; + + void reset(String source) { + this.source = source; + this.hashCode = 31 * source.hashCode() + DEFAULT_VARIANT; + } + + @Override + public int hashCode() { + return hashCode; + } + + @Override + public int variant() { + return DEFAULT_VARIANT; + } + + @Override + public boolean equals(Object other) { + if (other instanceof CharBuffer) { + CharBuffer otherBuffer = (CharBuffer) other; + if (otherBuffer.variant() == DEFAULT_VARIANT) { + int length = source.length(); + if (otherBuffer.length() == length) { + OfInt otherText = otherBuffer.chars(); + for (int i = 0; i < length; i++) { + if (source.charAt(i) != otherText.nextInt()) { + return false; + } + } + return true; + } + } + } + return false; + } + + @Override + public int length() { + return source.length(); + } + + @Override + public String toString() { + return source; + } + + @Override + public OfInt chars() { + return new OfInt() { + int index; + + @Override + public boolean hasNext() { + return index < source.length(); + } + + @Override + public int nextInt() { + return source.charAt(index++); + } + }; + } + + @Override + public String consume(BiConsumer valueConsumer) { + valueConsumer.accept(new CharBuffer.StringCharBuffer(source), source); + return source; + } + } } diff --git a/eureka-client/src/main/java/com/netflix/discovery/util/MapUtil.java b/eureka-client/src/main/java/com/netflix/discovery/util/MapUtil.java new file mode 100644 index 0000000000..182cdc28e7 --- /dev/null +++ b/eureka-client/src/main/java/com/netflix/discovery/util/MapUtil.java @@ -0,0 +1,43 @@ +/* + * Copyright 2026 Netflix, Inc. + * + * Licensed 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 com.netflix.discovery.util; + +import java.util.HashMap; + +/** + * Utility methods for working with Maps. + */ +public final class MapUtil { + + private MapUtil() { + // Utility class + } + + /** + * Creates a HashMap with enough capacity to hold the expected number of entries + * without resizing. This is equivalent to Guava's Maps.newHashMapWithExpectedSize(). + * + * @param expectedSize the expected number of entries + * @param the key type + * @param the value type + * @return a new HashMap with appropriate initial capacity + */ + public static HashMap newHashMapWithExpectedSize(int expectedSize) { + // HashMap resizes at load factor 0.75, so we need capacity = expectedSize / 0.75 + 1 + return new HashMap<>((int) (expectedSize / 0.75f) + 1); + } +} diff --git a/eureka-client/src/test/java/com/netflix/discovery/shared/ApplicationsTest.java b/eureka-client/src/test/java/com/netflix/discovery/shared/ApplicationsTest.java index a1f30c184e..d4e010cfe9 100644 --- a/eureka-client/src/test/java/com/netflix/discovery/shared/ApplicationsTest.java +++ b/eureka-client/src/test/java/com/netflix/discovery/shared/ApplicationsTest.java @@ -389,6 +389,659 @@ public DataCenterInfo.Name getName() { assertNotNull(applications.getRegisteredApplications("TestApp").getByInstanceId("test.hostname")); assertTrue(applications.getInstancesBySecureVirtualHostName("securetest.testname:7102").isEmpty()); assertTrue(applications.getInstancesBySecureVirtualHostName("test.testname:1").isEmpty()); - } + } + + // ==================== VIP Address Parsing Tests ==================== + + private static final DataCenterInfo TEST_DCI = () -> DataCenterInfo.Name.MyOwn; + + @Test + public void testVipParsing_nullVip() { + Application app = new Application("TestApp"); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + // No instances added + assertEquals(0, apps.getRegisteredApplications("TestApp").size()); + assertEquals(0, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // No VIP mappings exist + assertTrue(apps.getInstancesByVirtualHostName("anything").isEmpty()); + } + + @Test + public void testVipParsing_emptyVip() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + // Instance exists in application + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // Legacy behavior: empty VIP creates a mapping with empty string key + assertEquals(1, apps.getInstancesByVirtualHostName("").size()); + } + + @Test + public void testVipParsing_singleVip() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + assertEquals(1, apps.getInstancesByVirtualHostName("my.vip").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("MY.VIP").size()); // case-insensitive lookup + assertTrue(apps.getInstancesByVirtualHostName("other.vip").isEmpty()); // no other entries + } + + @Test + public void testVipParsing_singleVipMixedCase() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("My.Vip.Address").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // All case variations should resolve to same entry + assertEquals(1, apps.getInstancesByVirtualHostName("my.vip.address").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("MY.VIP.ADDRESS").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("My.Vip.Address").size()); + } + + @Test + public void testVipParsing_twoVips() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip1,vip2").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + // One instance in the application + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // Mapped to two VIPs + assertEquals(1, apps.getInstancesByVirtualHostName("vip1").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip2").size()); + // No spurious entries + assertTrue(apps.getInstancesByVirtualHostName("vip1,vip2").isEmpty()); + assertTrue(apps.getInstancesByVirtualHostName("vip3").isEmpty()); + } + + @Test + public void testVipParsing_threeVips() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip1,vip2,vip3").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip1").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip2").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip3").size()); + // No spurious entries from parsing + assertTrue(apps.getInstancesByVirtualHostName("vip1,vip2").isEmpty()); + assertTrue(apps.getInstancesByVirtualHostName("vip2,vip3").isEmpty()); + assertTrue(apps.getInstancesByVirtualHostName("vip1,vip2,vip3").isEmpty()); + } + + @Test + public void testVipParsing_multipleVipsMixedCase() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("Vip.One,VIP.TWO,vip.three").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // All stored as uppercase, lookup case-insensitive + assertEquals(1, apps.getInstancesByVirtualHostName("vip.one").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("VIP.ONE").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip.two").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip.three").size()); + } + + @Test + public void testVipParsing_multipleInstancesOverlappingVips() { + // host1: vip.one, vip.two, vip.three + // host2: vip.two, vip.four + // host3: vip.three, vip.four, vip.five + InstanceInfo host1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip.one,vip.two,vip.three").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo host2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip.two,vip.four").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + InstanceInfo host3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip.three,vip.four,vip.five").setDataCenterInfo(TEST_DCI) + .setHostName("host3").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(host1); + app.addInstance(host2); + app.addInstance(host3); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + // Verify application contains all 3 instances + assertEquals(3, apps.getRegisteredApplications("TestApp").size()); + assertEquals(3, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + + // vip.one -> host1 only + List vipOneInstances = apps.getInstancesByVirtualHostName("vip.one"); + assertEquals(1, vipOneInstances.size()); + assertEquals("host1", vipOneInstances.get(0).getHostName()); + + // vip.two -> host1, host2 + List vipTwoInstances = apps.getInstancesByVirtualHostName("vip.two"); + assertEquals(2, vipTwoInstances.size()); + assertTrue(vipTwoInstances.stream().anyMatch(i -> "host1".equals(i.getHostName()))); + assertTrue(vipTwoInstances.stream().anyMatch(i -> "host2".equals(i.getHostName()))); + + // vip.three -> host1, host3 + List vipThreeInstances = apps.getInstancesByVirtualHostName("vip.three"); + assertEquals(2, vipThreeInstances.size()); + assertTrue(vipThreeInstances.stream().anyMatch(i -> "host1".equals(i.getHostName()))); + assertTrue(vipThreeInstances.stream().anyMatch(i -> "host3".equals(i.getHostName()))); + + // vip.four -> host2, host3 + List vipFourInstances = apps.getInstancesByVirtualHostName("vip.four"); + assertEquals(2, vipFourInstances.size()); + assertTrue(vipFourInstances.stream().anyMatch(i -> "host2".equals(i.getHostName()))); + assertTrue(vipFourInstances.stream().anyMatch(i -> "host3".equals(i.getHostName()))); + + // vip.five -> host3 only + List vipFiveInstances = apps.getInstancesByVirtualHostName("vip.five"); + assertEquals(1, vipFiveInstances.size()); + assertEquals("host3", vipFiveInstances.get(0).getHostName()); + + // Case-insensitive lookups work + assertEquals(2, apps.getInstancesByVirtualHostName("VIP.TWO").size()); + assertEquals(2, apps.getInstancesByVirtualHostName("VIP.FOUR").size()); + + // No spurious VIP entries + assertTrue(apps.getInstancesByVirtualHostName("vip.six").isEmpty()); + assertTrue(apps.getInstancesByVirtualHostName("vip.one,vip.two").isEmpty()); + } + + // ==================== shuffleAndFilterInstances Tests ==================== + + @Test + public void testMultipleInstancesFiltering() { + DataCenterInfo myDCI = new DataCenterInfo() { + public DataCenterInfo.Name getName() { + return DataCenterInfo.Name.MyOwn; + } + }; + InstanceInfo up1 = InstanceInfo.Builder.newBuilder().setAppName("test") + .setVIPAddress("test.vip").setDataCenterInfo(myDCI) + .setHostName("up1.hostname").setStatus(InstanceStatus.UP).build(); + InstanceInfo up2 = InstanceInfo.Builder.newBuilder().setAppName("test") + .setVIPAddress("test.vip").setDataCenterInfo(myDCI) + .setHostName("up2.hostname").setStatus(InstanceStatus.UP).build(); + InstanceInfo down = InstanceInfo.Builder.newBuilder().setAppName("test") + .setVIPAddress("test.vip").setDataCenterInfo(myDCI) + .setHostName("down.hostname").setStatus(InstanceStatus.DOWN).build(); + + Application application = new Application("TestApp"); + application.addInstance(up1); + application.addInstance(up2); + application.addInstance(down); + + Applications applications = new Applications(); + applications.addApplication(application); + applications.shuffleInstances(true); + + List result = applications.getInstancesByVirtualHostName("test.vip"); + assertEquals(2, result.size()); + assertTrue(result.contains(up1)); + assertTrue(result.contains(up2)); + assertFalse(result.contains(down)); + } + + @Test + public void testSingleInstanceUp_filterEnabled() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(instance)); + } + + @Test + public void testSingleInstanceDown_filterEnabled() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.DOWN).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + // Single DOWN instance should be filtered out + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertTrue(result.isEmpty()); + // But instance still exists in the application + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + } + + @Test + public void testMultipleInstances_filterDisabled() { + InstanceInfo up = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up.host").setStatus(InstanceStatus.UP).build(); + InstanceInfo down = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down.host").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo oos = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("oos.host").setStatus(InstanceStatus.OUT_OF_SERVICE).build(); + + Application app = new Application("TestApp"); + app.addInstance(up); + app.addInstance(down); + app.addInstance(oos); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); // filterUpInstances = false + + // All instances should be present regardless of status + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(3, result.size()); + assertTrue(result.contains(up)); + assertTrue(result.contains(down)); + assertTrue(result.contains(oos)); + } + + @Test + public void testAllInstancesNonUp_filterEnabled() { + InstanceInfo down = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down.host").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo oos = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("oos.host").setStatus(InstanceStatus.OUT_OF_SERVICE).build(); + InstanceInfo starting = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("starting.host").setStatus(InstanceStatus.STARTING).build(); + + Application app = new Application("TestApp"); + app.addInstance(down); + app.addInstance(oos); + app.addInstance(starting); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + // All instances filtered out + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertTrue(result.isEmpty()); + // But all instances still exist in the application + assertEquals(3, apps.getRegisteredApplications("TestApp").size()); + } + + @Test + public void testSecureVipFiltering() { + InstanceInfo up = InstanceInfo.Builder.newBuilder() + .setAppName("test").setSecureVIPAddress("secure.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up.host").setStatus(InstanceStatus.UP).build(); + InstanceInfo down = InstanceInfo.Builder.newBuilder() + .setAppName("test").setSecureVIPAddress("secure.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down.host").setStatus(InstanceStatus.DOWN).build(); + + Application app = new Application("TestApp"); + app.addInstance(up); + app.addInstance(down); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesBySecureVirtualHostName("secure.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(up)); + assertFalse(result.contains(down)); + } + + @Test + public void testReshuffleUpdatesFilteredList() { + InstanceInfo host1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo host2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(host1); + app.addInstance(host2); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + assertEquals(2, apps.getInstancesByVirtualHostName("my.vip").size()); + + // Change host2 to DOWN and reshuffle + host2.setStatus(InstanceStatus.DOWN); + apps.shuffleInstances(true); + + // Only host1 should remain + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(host1)); + assertFalse(result.contains(host2)); + } + + @Test + public void testMixedVipAndSecureVipFiltering() { + InstanceInfo upBoth = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setSecureVIPAddress("secure.vip") + .setDataCenterInfo(TEST_DCI).setHostName("up.both").setStatus(InstanceStatus.UP).build(); + InstanceInfo downBoth = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setSecureVIPAddress("secure.vip") + .setDataCenterInfo(TEST_DCI).setHostName("down.both").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo upVipOnly = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip") + .setDataCenterInfo(TEST_DCI).setHostName("up.vip").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(upBoth); + app.addInstance(downBoth); + app.addInstance(upVipOnly); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + // VIP should have 2 UP instances + List vipResult = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(2, vipResult.size()); + assertTrue(vipResult.contains(upBoth)); + assertTrue(vipResult.contains(upVipOnly)); + + // Secure VIP should have 1 UP instance + List secureResult = apps.getInstancesBySecureVirtualHostName("secure.vip"); + assertEquals(1, secureResult.size()); + assertTrue(secureResult.contains(upBoth)); + } + + // ==================== Collection Progression Tests (emptyList -> singletonList -> ArrayList) ==================== + + @Test + public void testVipInstanceCountProgression() { + // Explicitly test 0->1->2->3->4 instance progression on same VIP + // Validates emptyList -> singletonList -> ArrayList(12) transitions + InstanceInfo inst1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host3").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst4 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host4").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + Applications apps = new Applications(); + + // 0 instances - no VIP entry exists yet + apps.addApplication(app); + apps.shuffleInstances(false); + assertTrue(apps.getInstancesByVirtualHostName("my.vip").isEmpty()); + + // 1 instance - singletonList path + app.addInstance(inst1); + apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + List result1 = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result1.size()); + assertTrue(result1.contains(inst1)); + + // 2 instances - ArrayList transition + app.addInstance(inst2); + apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + List result2 = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(2, result2.size()); + assertTrue(result2.contains(inst1)); + assertTrue(result2.contains(inst2)); + + // 3 instances - ArrayList grows + app.addInstance(inst3); + apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + List result3 = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(3, result3.size()); + + // 4 instances - ArrayList continues to grow + app.addInstance(inst4); + apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + List result4 = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(4, result4.size()); + assertTrue(result4.contains(inst1)); + assertTrue(result4.contains(inst2)); + assertTrue(result4.contains(inst3)); + assertTrue(result4.contains(inst4)); + } + + @Test + public void testFilteringWithSingletonList_instanceUp() { + // Single UP instance: singletonList should be preserved + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(instance)); + } + + @Test + public void testFilteringWithSingletonList_instanceDown() { + // Single DOWN instance: should return emptyList + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.DOWN).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertTrue(result.isEmpty()); + } + + @Test + public void testFilteringWithArrayList_oneUpOneDown() { + // 2 instances (ArrayList), 1 UP 1 DOWN: filters to 1 + InstanceInfo up = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up.host").setStatus(InstanceStatus.UP).build(); + InstanceInfo down = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down.host").setStatus(InstanceStatus.DOWN).build(); + Application app = new Application("TestApp"); + app.addInstance(up); + app.addInstance(down); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(up)); + assertFalse(result.contains(down)); + } + + @Test + public void testFilteringWithArrayList_allDown() { + // 3 instances (ArrayList), all DOWN: should return emptyList + InstanceInfo down1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down1.host").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo down2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down2.host").setStatus(InstanceStatus.OUT_OF_SERVICE).build(); + InstanceInfo down3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down3.host").setStatus(InstanceStatus.STARTING).build(); + Application app = new Application("TestApp"); + app.addInstance(down1); + app.addInstance(down2); + app.addInstance(down3); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertTrue(result.isEmpty()); + } + + @Test + public void testFilteringPreservesUpInstancesInOrder() { + // Verify UP instances are preserved (order may change due to shuffle) + InstanceInfo up1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up1").setStatus(InstanceStatus.UP).build(); + InstanceInfo down1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down1").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo up2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up2").setStatus(InstanceStatus.UP).build(); + InstanceInfo down2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down2").setStatus(InstanceStatus.OUT_OF_SERVICE).build(); + InstanceInfo up3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up3").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(up1); + app.addInstance(down1); + app.addInstance(up2); + app.addInstance(down2); + app.addInstance(up3); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(3, result.size()); + assertTrue(result.contains(up1)); + assertTrue(result.contains(up2)); + assertTrue(result.contains(up3)); + assertFalse(result.contains(down1)); + assertFalse(result.contains(down2)); + } + + @Test + public void testInstanceRemovalFromApplication() { + // Test that removing an instance from application is reflected after reshuffle + InstanceInfo inst1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host3").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(inst1); + app.addInstance(inst2); + app.addInstance(inst3); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(3, apps.getInstancesByVirtualHostName("my.vip").size()); + + // Remove one instance + app.removeInstance(inst2); + apps.shuffleInstances(false); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(2, result.size()); + assertTrue(result.contains(inst1)); + assertFalse(result.contains(inst2)); + assertTrue(result.contains(inst3)); + } + + @Test + public void testRemoveAllInstancesFromVip() { + // Test removing all instances results in empty VIP list + InstanceInfo inst1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(inst1); + app.addInstance(inst2); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(2, apps.getInstancesByVirtualHostName("my.vip").size()); + + // Remove all instances + app.removeInstance(inst1); + app.removeInstance(inst2); + apps.shuffleInstances(false); + + assertTrue(apps.getInstancesByVirtualHostName("my.vip").isEmpty()); + } } diff --git a/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java b/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java index 6cf9b18d7f..cfd1bac254 100644 --- a/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java +++ b/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java @@ -2,60 +2,239 @@ import java.io.IOException; import java.util.Arrays; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; +import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectReader; import com.netflix.discovery.util.DeserializerStringCache.CacheScope; import org.junit.Test; import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class DeserializerStringCacheTest { + private static final JsonFactory JSON_FACTORY = new JsonFactory(); + + private static JsonParser createParser(String jsonValue) throws IOException { + // Create a real parser positioned at a string value + JsonParser parser = JSON_FACTORY.createParser("\"" + jsonValue + "\""); + parser.nextToken(); // Move to VALUE_STRING + return parser; + } + + private DeserializerStringCache createCache() { + ObjectReader reader = DeserializerStringCache.init(new ObjectMapper().reader()); + return (DeserializerStringCache) reader.getAttributes().getAttribute("deserInternCache"); + } + @Test public void testUppercaseConversionWithLowercasePreset() throws IOException { - DeserializationContext deserializationContext = mock(DeserializationContext.class); - DeserializerStringCache deserializerStringCache = DeserializerStringCache.from(deserializationContext); + DeserializerStringCache cache = createCache(); - String lowerCaseValue = deserializerStringCache.apply("value", CacheScope.APPLICATION_SCOPE); + String lowerCaseValue = cache.apply("value", CacheScope.APPLICATION_SCOPE); assertThat(lowerCaseValue, is("value")); - JsonParser jsonParser = mock(JsonParser.class); - when(jsonParser.getTextCharacters()).thenReturn(new char[] {'v', 'a', 'l', 'u', 'e'}); - when(jsonParser.getTextLength()).thenReturn(5); - - String upperCaseValue = deserializerStringCache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, () -> "VALUE"); - assertThat(upperCaseValue, is("VALUE")); + try (JsonParser jsonParser = createParser("value")) { + String upperCaseValue = cache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, jp -> "VALUE"); + assertThat(upperCaseValue, is("VALUE")); + } } @Test public void testUppercaseConversionWithLongString() throws IOException { - DeserializationContext deserializationContext = mock(DeserializationContext.class); - DeserializerStringCache deserializerStringCache = DeserializerStringCache.from(deserializationContext); + DeserializerStringCache cache = createCache(); char[] lowercaseValue = new char[1024]; Arrays.fill(lowercaseValue, 'a'); + String longString = new String(lowercaseValue); + + try (JsonParser jsonParser = createParser(longString)) { + char[] expectedValueChars = new char[1024]; + Arrays.fill(expectedValueChars, 'A'); + String expectedValue = new String(expectedValueChars); + + String upperCaseValue = cache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, + jp -> longString.toUpperCase()); + assertThat(upperCaseValue, is(expectedValue)); + } + } + + @Test + public void testCacheHitReturnsIdenticalInstance() throws IOException { + DeserializerStringCache cache = createCache(); + + String first; + try (JsonParser p1 = createParser("testValue")) { + first = cache.apply(p1, CacheScope.APPLICATION_SCOPE); + } + + String second; + try (JsonParser p2 = createParser("testValue")) { + second = cache.apply(p2, CacheScope.APPLICATION_SCOPE); + } + + assertSame("Cache hit should return identical instance", first, second); + } - JsonParser jsonParser = mock(JsonParser.class); - when(jsonParser.getText()).thenReturn(new String(lowercaseValue)); - when(jsonParser.getTextCharacters()).thenReturn(lowercaseValue); - when(jsonParser.getTextOffset()).thenReturn(0); - when(jsonParser.getTextLength()).thenReturn(lowercaseValue.length); - - String upperCaseValue = deserializerStringCache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, () -> { - try { - return jsonParser.getText().toUpperCase(); - } - catch(IOException ioe) { - // not likely from mock above - throw new IllegalStateException("mock threw unexpected exception", ioe); - } - }); - char[] expectedValueChars = new char[1024]; - Arrays.fill(expectedValueChars, 'A'); - String expectedValue = new String(expectedValueChars); - assertThat(upperCaseValue, is(expectedValue)); - } -} \ No newline at end of file + @Test + public void testCacheHitWithStringReturnsIdenticalInstance() throws IOException { + DeserializerStringCache cache = createCache(); + + String first = cache.apply(new String("testValue"), CacheScope.APPLICATION_SCOPE); + String second = cache.apply(new String("testValue"), CacheScope.APPLICATION_SCOPE); + + assertSame("Cache hit should return identical instance", first, second); + } + + @Test + public void testCacheHitAcrossParserAndString() throws IOException { + DeserializerStringCache cache = createCache(); + + String fromParser; + try (JsonParser parser = createParser("testValue")) { + fromParser = cache.apply(parser, CacheScope.APPLICATION_SCOPE); + } + String fromString = cache.apply(new String("testValue"), CacheScope.APPLICATION_SCOPE); + + assertSame("Cache should work across parser and string lookups", fromParser, fromString); + } + + @Test + public void testTransformOnlyCalledOnCacheMiss() throws IOException { + DeserializerStringCache cache = createCache(); + AtomicInteger callCount = new AtomicInteger(0); + + Function countingTransform = jp -> { + callCount.incrementAndGet(); + return "TRANSFORMED"; + }; + + try (JsonParser p1 = createParser("value")) { + cache.apply(p1, CacheScope.APPLICATION_SCOPE, countingTransform); + } + try (JsonParser p2 = createParser("value")) { + cache.apply(p2, CacheScope.APPLICATION_SCOPE, countingTransform); + } + + assertEquals("Transform should only be called once (on cache miss)", 1, callCount.get()); + } + + @Test + public void testGlobalScopeSurvivesApplicationScopeClear() throws IOException { + ObjectReader reader = DeserializerStringCache.init(new ObjectMapper().reader()); + DeserializerStringCache cache = (DeserializerStringCache) reader.getAttributes() + .getAttribute("deserInternCache"); + + String globalValue; + try (JsonParser p1 = createParser("globalKey")) { + globalValue = cache.apply(p1, CacheScope.GLOBAL_SCOPE); + } + + String appValue; + try (JsonParser p2 = createParser("appKey")) { + appValue = cache.apply(p2, CacheScope.APPLICATION_SCOPE); + } + + // Clear only application scope + DeserializerStringCache.clear(reader, CacheScope.APPLICATION_SCOPE); + + // Global should still return same instance + String globalAgain; + try (JsonParser p3 = createParser("globalKey")) { + globalAgain = cache.apply(p3, CacheScope.GLOBAL_SCOPE); + } + assertSame("Global value should survive application scope clear", globalValue, globalAgain); + + // Application scope was cleared, so this should be a new instance + String appAgain; + try (JsonParser p4 = createParser("appKey")) { + appAgain = cache.apply(p4, CacheScope.APPLICATION_SCOPE); + } + assertNotSame("Application value should be new after clear", appValue, appAgain); + assertEquals("Application value should have same content", appValue, appAgain); + } + + @Test + public void testGlobalScopeClearClearsBothScopes() throws IOException { + ObjectReader reader = DeserializerStringCache.init(new ObjectMapper().reader()); + DeserializerStringCache cache = (DeserializerStringCache) reader.getAttributes() + .getAttribute("deserInternCache"); + + String globalValue; + try (JsonParser p1 = createParser("globalKey")) { + globalValue = cache.apply(p1, CacheScope.GLOBAL_SCOPE); + } + + String appValue; + try (JsonParser p2 = createParser("appKey")) { + appValue = cache.apply(p2, CacheScope.APPLICATION_SCOPE); + } + + // Clear global scope (should clear both) + DeserializerStringCache.clear(reader, CacheScope.GLOBAL_SCOPE); + + String globalAgain; + try (JsonParser p3 = createParser("globalKey")) { + globalAgain = cache.apply(p3, CacheScope.GLOBAL_SCOPE); + } + + String appAgain; + try (JsonParser p4 = createParser("appKey")) { + appAgain = cache.apply(p4, CacheScope.APPLICATION_SCOPE); + } + + assertNotSame("Global value should be new after global clear", globalValue, globalAgain); + assertNotSame("Application value should be new after global clear", appValue, appAgain); + } + + @Test + public void testParserWithNonZeroOffset() throws IOException { + DeserializerStringCache cache = createCache(); + + // First cache "value" from a normal parse + String cached; + try (JsonParser p1 = createParser("value")) { + cached = cache.apply(p1, CacheScope.APPLICATION_SCOPE); + } + assertEquals("Should extract correct value", "value", cached); + + // Verify same value from different parse returns cached instance + String cachedAgain; + try (JsonParser p2 = createParser("value")) { + cachedAgain = cache.apply(p2, CacheScope.APPLICATION_SCOPE); + } + assertSame("Should match cache entry", cached, cachedAgain); + } + + @Test + public void testDifferentTransformsForSameKeyAreCachedSeparately() throws IOException { + DeserializerStringCache cache = createCache(); + + // Same raw key "app" but different transforms (identity vs toUpperCase) + String lowercase; + try (JsonParser p1 = createParser("app")) { + lowercase = cache.apply(p1, CacheScope.APPLICATION_SCOPE); + } + + // Use a different function class - this should create a different cache entry + // because the variant is based on the function class identity + class UpperCaseTransform implements Function { + public String apply(JsonParser jp) { return "APP"; } + } + + String uppercase; + try (JsonParser p2 = createParser("app")) { + uppercase = cache.apply(p2, CacheScope.APPLICATION_SCOPE, new UpperCaseTransform()); + } + + assertEquals("Lowercase should be 'app'", "app", lowercase); + assertEquals("Uppercase should be 'APP'", "APP", uppercase); + assertNotSame("Different transforms should cache separately", lowercase, uppercase); + } +} diff --git a/eureka-core/src/main/java/com/netflix/eureka/registry/RemoteRegionRegistry.java b/eureka-core/src/main/java/com/netflix/eureka/registry/RemoteRegionRegistry.java index 130e0e97ad..20753fad1b 100644 --- a/eureka-core/src/main/java/com/netflix/eureka/registry/RemoteRegionRegistry.java +++ b/eureka-core/src/main/java/com/netflix/eureka/registry/RemoteRegionRegistry.java @@ -235,10 +235,10 @@ private boolean fetchRegistry() { // If the delta is disabled or if it is the first time, get all applications if (serverConfig.shouldDisableDeltaForRemoteRegions() || (getApplications() == null) - || (getApplications().getRegisteredApplications().size() == 0)) { + || getApplications().isRegisteredApplicationsEmpty()) { logger.info("Disable delta property : {}", serverConfig.shouldDisableDeltaForRemoteRegions()); logger.info("Application is null : {}", getApplications() == null); - logger.info("Registered Applications size is zero : {}", getApplications().getRegisteredApplications().isEmpty()); + logger.info("Registered Applications size is zero : {}", getApplications().isRegisteredApplicationsEmpty()); success = storeFullRegistry(); } else { success = fetchAndStoreDelta();