From 4ab4e2274821aff4c51df3608439c894f9feb910 Mon Sep 17 00:00:00 2001 From: Bohan Yang Date: Fri, 14 Jun 2024 12:24:41 -0700 Subject: [PATCH 1/2] add warn logs for receiving null properties --- .../d2/balancer/simple/ClusterLoadBalancerSubscriber.java | 1 + .../d2/balancer/simple/ServiceLoadBalancerSubscriber.java | 2 +- .../linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java | 2 +- .../java/com/linkedin/d2/discovery/stores/file/FileStore.java | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/d2/src/main/java/com/linkedin/d2/balancer/simple/ClusterLoadBalancerSubscriber.java b/d2/src/main/java/com/linkedin/d2/balancer/simple/ClusterLoadBalancerSubscriber.java index 98b90a3332..a1329adf8c 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/simple/ClusterLoadBalancerSubscriber.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/simple/ClusterLoadBalancerSubscriber.java @@ -77,6 +77,7 @@ protected void handlePut(final String listenTo, final ClusterProperties discover } else { + _log.warn("Received a null cluster properties for {}", listenTo); // still insert the ClusterInfoItem when discoveryProperties is null, but don't create accessor _simpleLoadBalancerState.getClusterInfo().put(listenTo, new ClusterInfoItem(_simpleLoadBalancerState, null, null, null)); diff --git a/d2/src/main/java/com/linkedin/d2/balancer/simple/ServiceLoadBalancerSubscriber.java b/d2/src/main/java/com/linkedin/d2/balancer/simple/ServiceLoadBalancerSubscriber.java index 37cd144797..422afc6297 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/simple/ServiceLoadBalancerSubscriber.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/simple/ServiceLoadBalancerSubscriber.java @@ -122,7 +122,7 @@ else if (oldServicePropertiesItem != null) // we'll just ignore the event and move on. // we could receive a null if the file store properties cannot read/write a file. // in this case it's better to leave the state intact and not do anything - _log.warn("We receive a null service properties for {}. ", listenTo); + _log.warn("Received a null service properties for {}", listenTo); } } diff --git a/d2/src/main/java/com/linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java b/d2/src/main/java/com/linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java index ae41f5258a..98f6bd4790 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java @@ -164,7 +164,7 @@ protected void handlePut(final String cluster, final UriProperties uriProperties // uri properties was null, we'll just log the event and continues. // The reasoning is we might receive a null event when there's a problem writing/reading // cache file, or we just started listening to a cluster without any uris yet. - warn(_log, "received a null uri properties for cluster: ", cluster); + _log.warn("Received a null uri properties for cluster: {}", cluster); } } diff --git a/d2/src/main/java/com/linkedin/d2/discovery/stores/file/FileStore.java b/d2/src/main/java/com/linkedin/d2/discovery/stores/file/FileStore.java index bcad5deaf0..1fba8c52b2 100644 --- a/d2/src/main/java/com/linkedin/d2/discovery/stores/file/FileStore.java +++ b/d2/src/main/java/com/linkedin/d2/discovery/stores/file/FileStore.java @@ -212,7 +212,7 @@ public void put(String listenTo, T discoveryProperties) { if (discoveryProperties == null) { - warn(_log, "received a null property for resource ", listenTo, " received a null property"); + _log.warn("Received and ignored a null property for resource: {}", listenTo); } else { From 465a0d418b03efbd013471efa820f66af9561b8c Mon Sep 17 00:00:00 2001 From: Bohan Yang Date: Fri, 14 Jun 2024 13:53:08 -0700 Subject: [PATCH 2/2] use rate limited logger for uri logs --- .../d2/balancer/simple/UriLoadBalancerSubscriber.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/d2/src/main/java/com/linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java b/d2/src/main/java/com/linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java index 98f6bd4790..86078a8756 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/simple/UriLoadBalancerSubscriber.java @@ -22,12 +22,15 @@ import com.linkedin.d2.balancer.properties.PartitionData; import com.linkedin.d2.balancer.properties.UriProperties; import com.linkedin.d2.discovery.event.PropertyEventBus; +import com.linkedin.util.RateLimitedLogger; +import com.linkedin.util.clock.SystemClock; import java.net.URI; import java.util.Iterator; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,6 +43,8 @@ class UriLoadBalancerSubscriber extends AbstractLoadBalancerSubscriber { private static final Logger _log = LoggerFactory.getLogger(UriLoadBalancerSubscriber.class); + private static final RateLimitedLogger RATE_LIMITED_LOGGER = + new RateLimitedLogger(_log, TimeUnit.MINUTES.toMillis(10), SystemClock.instance()); private SimpleLoadBalancerState _simpleLoadBalancerState; @@ -164,7 +169,7 @@ protected void handlePut(final String cluster, final UriProperties uriProperties // uri properties was null, we'll just log the event and continues. // The reasoning is we might receive a null event when there's a problem writing/reading // cache file, or we just started listening to a cluster without any uris yet. - _log.warn("Received a null uri properties for cluster: {}", cluster); + RATE_LIMITED_LOGGER.warn("Received a null uri properties for cluster: {}", cluster); } } @@ -172,7 +177,7 @@ protected void handlePut(final String cluster, final UriProperties uriProperties protected void handleRemove(final String cluster) { _simpleLoadBalancerState.getUriProperties().remove(cluster); - warn(_log, "received a uri properties event remove() for cluster: ", cluster); + warn(RATE_LIMITED_LOGGER, "received a uri properties event remove() for cluster: ", cluster); _simpleLoadBalancerState.removeTrackerClients(cluster); } }