This is an automated email from the ASF dual-hosted git repository.

earthchen pushed a commit to branch 3.3
in repository https://gitbox.apache.org/repos/asf/dubbo.git


The following commit(s) were added to refs/heads/3.3 by this push:
     new 0f3ce734b4 Fix timestamp filtering in ZooKeeper registry causing 
pre-warmup failure (#16034)
0f3ce734b4 is described below

commit 0f3ce734b435429f02594bcbc8d1699bdd2ca923
Author: funkye <[email protected]>
AuthorDate: Fri Jan 23 11:29:45 2026 +0800

    Fix timestamp filtering in ZooKeeper registry causing pre-warmup failure 
(#16034)
    
    * Fix timestamp filtering in ZooKeeper registry causing pre-warmup failure
    
    * Refactor CacheableFailbackRegistry to streamline URL creation and improve 
cache handling
    
    * Refactor CacheableFailbackRegistry to streamline URL creation and improve 
cache handling
    
    ---------
    
    Co-authored-by: Rain Yu <[email protected]>
---
 .../support/CacheableFailbackRegistry.java         | 115 ++++++++++++---
 .../support/CacheableFailbackRegistryTest.java     | 162 +++++++++++++++++++++
 2 files changed, 254 insertions(+), 23 deletions(-)

diff --git 
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/CacheableFailbackRegistry.java
 
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/CacheableFailbackRegistry.java
index 41b2c3ac4b..43e636fccc 100644
--- 
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/CacheableFailbackRegistry.java
+++ 
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/CacheableFailbackRegistry.java
@@ -37,6 +37,7 @@ import org.apache.dubbo.rpc.model.ScopeModel;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -176,6 +177,8 @@ public abstract class CacheableFailbackRegistry extends 
FailbackRegistry {
         Map<String, ServiceAddressURL> oldURLs = stringUrls.get(consumer);
 
         // create new urls
+        // The key of newURLs is the normalized rawProvider (variable keys 
removed for deduplication),
+        // but the cached ServiceAddressURL is built from the original 
rawProvider (with all parameters preserved).
         Map<String, ServiceAddressURL> newURLs = new HashMap<>((int) 
(providers.size() / 0.75f + 1));
 
         // remove 'release', 'dubbo', 'methods', timestamp, 'dubbo.tag' 
parameter
@@ -184,28 +187,42 @@ public abstract class CacheableFailbackRegistry extends 
FailbackRegistry {
 
         if (oldURLs == null) {
             for (String rawProvider : providers) {
-                // remove VARIABLE_KEYS(timestamp,pid..) in provider url.
-                rawProvider = stripOffVariableKeys(rawProvider);
-
-                // create DubboServiceAddress object using provider url, 
consumer url, and extra parameters.
+                // Normalize the rawProvider by removing 
VARIABLE_KEYS(timestamp, pid..) for deduplication purposes.
+                // The normalized key is used to avoid duplicate instances of 
the same provider.
+                String normalizedKey = stripOffVariableKeys(rawProvider);
+
+                // Create DubboServiceAddress object using the ORIGINAL 
rawProvider (with all parameters intact),
+                // so that timestamp and other parameters are preserved for 
correct parameter parsing.
+                // This ensures that when multiple providers normalize to the 
same key (e.g., different timestamps),
+                // the latest one's timestamp will be used.
                 ServiceAddressURL cachedURL = createURL(rawProvider, 
copyOfConsumer, getExtraParameters());
                 if (cachedURL == null) {
                     continue;
                 }
-                newURLs.put(rawProvider, cachedURL);
+
+                // Use normalized key for deduplication: if multiple providers 
normalize to the same key,
+                // the last one (with the latest timestamp) will be kept.
+                newURLs.put(normalizedKey, cachedURL);
             }
         } else {
-            // maybe only default, or "env" + default
+            // Reuse or create URLs based on both normalized key and original 
rawProvider.
+            // The normalized key is used to find potential cache entries for 
reuse (deduplication),
+            // but we always create a new ServiceAddressURL from the original 
rawProvider to ensure
+            // the timestamp and other parameters are up-to-date.
             for (String rawProvider : providers) {
-                rawProvider = stripOffVariableKeys(rawProvider);
-                ServiceAddressURL cachedURL = oldURLs.remove(rawProvider);
+                // Normalize the rawProvider for cache key matching and 
deduplication.
+                String normalizedKey = stripOffVariableKeys(rawProvider);
+                ServiceAddressURL cachedURL = oldURLs.remove(normalizedKey);
                 if (cachedURL == null) {
+                    // Create new URL using the original rawProvider (all 
parameters preserved including timestamp).
                     cachedURL = createURL(rawProvider, copyOfConsumer, 
getExtraParameters());
                     if (cachedURL == null) {
                         continue;
                     }
                 }
-                newURLs.put(rawProvider, cachedURL);
+                // Use normalized key for storage: if multiple providers 
normalize to the same key,
+                // the last one will be kept, ensuring no duplicate instances 
with outdated timestamps.
+                newURLs.put(normalizedKey, cachedURL);
             }
         }
 
@@ -323,29 +340,81 @@ public abstract class CacheableFailbackRegistry extends 
FailbackRegistry {
             return rawProvider;
         }
 
+        int encodedIdx = rawProvider.indexOf(ENCODED_QUESTION_MARK);
+        int plainIdx = rawProvider.indexOf('?');
+        boolean encoded;
+        int questionIdx;
+        if (encodedIdx >= 0 && (plainIdx < 0 || encodedIdx < plainIdx)) {
+            encoded = true;
+            questionIdx = encodedIdx;
+        } else if (plainIdx >= 0) {
+            encoded = false;
+            questionIdx = plainIdx;
+        } else {
+            return rawProvider;
+        }
+
+        String prefix = rawProvider.substring(0, questionIdx);
+        String params = rawProvider.substring(questionIdx + (encoded ? 
ENCODED_QUESTION_MARK.length() : 1));
+        if (params.isEmpty()) {
+            return rawProvider;
+        }
+
+        String andMark = encoded ? ENCODED_AND_MARK : "&";
+        String eqMark = encoded ? "%3D" : "=";
+
+        // Build a set of normalized variable names for efficient matching.
+        HashSet<String> variableNames = new HashSet<>(keys.length);
         for (String key : keys) {
-            int idxStart = rawProvider.indexOf(key);
-            if (idxStart == -1) {
+            if (key != null) {
+                variableNames.add(normalizeVariableName(key));
+            }
+        }
+
+        List<String> remaining = new ArrayList<>();
+        boolean removed = false;
+        for (String param : params.split(andMark)) {
+            if (param.isEmpty()) {
                 continue;
             }
-            int idxEnd = rawProvider.indexOf(ENCODED_AND_MARK, idxStart);
-            String part1 = rawProvider.substring(0, idxStart);
-            if (idxEnd == -1) {
-                rawProvider = part1;
-            } else {
-                String part2 = rawProvider.substring(idxEnd + 
ENCODED_AND_MARK.length());
-                rawProvider = part1 + part2;
+            int eqIdx = param.indexOf(eqMark);
+            if (eqIdx < 0) {
+                remaining.add(param);
+                continue;
+            }
+            String name = param.substring(0, eqIdx);
+            String normalizedName = normalizeVariableName(name);
+
+            // Check if this parameter name exactly matches a variable key 
that should be removed.
+            // Only exact match is performed (e.g., "timestamp" matches 
"timestamp", but not "remote.timestamp").
+            if (variableNames.contains(normalizedName)) {
+                removed = true;
+                continue;
             }
+            remaining.add(param);
         }
 
-        if (rawProvider.endsWith(ENCODED_AND_MARK)) {
-            rawProvider = rawProvider.substring(0, rawProvider.length() - 
ENCODED_AND_MARK.length());
+        if (!removed) {
+            return rawProvider;
         }
-        if (rawProvider.endsWith(ENCODED_QUESTION_MARK)) {
-            rawProvider = rawProvider.substring(0, rawProvider.length() - 
ENCODED_QUESTION_MARK.length());
+        if (remaining.isEmpty()) {
+            return prefix;
         }
 
-        return rawProvider;
+        return prefix + (encoded ? ENCODED_QUESTION_MARK : "?") + 
String.join(andMark, remaining);
+    }
+
+    private String normalizeVariableName(String key) {
+        if (key == null) {
+            return null;
+        }
+        if (key.endsWith("%3D")) {
+            return key.substring(0, key.length() - 3);
+        }
+        if (key.endsWith("=")) {
+            return key.substring(0, key.length() - 1);
+        }
+        return key;
     }
 
     private List<URL> toConfiguratorsWithoutEmpty(URL consumer, 
Collection<String> configurators) {
diff --git 
a/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/support/CacheableFailbackRegistryTest.java
 
b/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/support/CacheableFailbackRegistryTest.java
new file mode 100644
index 0000000000..e4f19dfe73
--- /dev/null
+++ 
b/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/support/CacheableFailbackRegistryTest.java
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.dubbo.registry.support;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.url.component.ServiceAddressURL;
+import org.apache.dubbo.registry.NotifyListener;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.apache.dubbo.common.URLStrParser.ENCODED_AND_MARK;
+import static org.apache.dubbo.common.URLStrParser.ENCODED_QUESTION_MARK;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class CacheableFailbackRegistryTest {
+
+    private StubCacheableFailbackRegistry registry;
+
+    @BeforeEach
+    void setUp() {
+        registry = new 
StubCacheableFailbackRegistry(URL.valueOf("mock://127.0.0.1"));
+    }
+
+    @Test
+    void shouldRemoveExactTimestamp() throws Exception {
+        // Test removing exact "timestamp" parameter
+        String rawProvider = 
"dubbo://127.0.0.1:20880/demo.Service?timestamp=100&revision=v1";
+        String result = strip(rawProvider);
+        // After removing timestamp, only revision should remain
+        assertTrue(result.contains("revision=v1"), "Result should contain 
revision");
+        assertFalse(result.contains("timestamp"), "Result should not contain 
timestamp");
+    }
+
+    @Test
+    void shouldRemoveExactPid() throws Exception {
+        // Test removing exact "pid" parameter
+        String rawProvider = 
"dubbo://127.0.0.1:20880/demo.Service?pid=9999&revision=v1";
+        String result = strip(rawProvider);
+        assertTrue(result.contains("revision=v1"), "Result should contain 
revision");
+        assertFalse(result.contains("pid"), "Result should not contain pid");
+    }
+
+    @Test
+    void shouldRemoveTimestampAndPid() throws Exception {
+        // Test removing both timestamp and pid
+        String rawProvider = 
"dubbo://127.0.0.1:20880/demo.Service?timestamp=100&pid=1234&revision=abc";
+        String result = strip(rawProvider);
+        assertTrue(result.contains("revision=abc"), "Result should contain 
revision");
+        assertFalse(result.contains("timestamp"), "Result should not contain 
timestamp");
+        assertFalse(result.contains("pid"), "Result should not contain pid");
+    }
+
+    @Test
+    void shouldKeepRemoteTimestamp() throws Exception {
+        // remote.timestamp should NOT be removed (only exact "timestamp" is 
removed)
+        String rawProvider = 
"dubbo://127.0.0.1:20880/demo.Service?remote.timestamp=200&revision=v1";
+        String result = strip(rawProvider);
+        assertTrue(result.contains("remote.timestamp=200"), "Result should 
keep remote.timestamp");
+        assertTrue(result.contains("revision=v1"), "Result should contain 
revision");
+    }
+
+    @Test
+    void shouldRemoveEncodedTimestampAndPid() throws Exception {
+        // Test with encoded format
+        String base = "dubbo%3A%2F%2F127.0.0.1%3A20880%2Fdemo.Service";
+        String rawProvider = base + ENCODED_QUESTION_MARK
+                + "timestamp%3D123" + ENCODED_AND_MARK
+                + "pid%3D9999" + ENCODED_AND_MARK
+                + "revision%3Dv1";
+        String result = strip(rawProvider);
+        assertTrue(result.contains("revision%3Dv1"), "Result should contain 
revision");
+        assertFalse(result.contains("timestamp%3D"), "Result should not 
contain timestamp");
+        assertFalse(result.contains("pid%3D"), "Result should not contain 
pid");
+    }
+
+    @Test
+    void toUrlsWithoutEmptyPreservesTimestampInCachedURL() throws Exception {
+        // This test verifies the fix: normalized key for cache, but original 
rawProvider for URL building.
+        String consumerUrl = "mock://127.0.0.1/demo.Service";
+        String provider1 = 
"dubbo://provider1.example.com:20880/demo.Service?timestamp=100&revision=v1";
+        String provider2 = 
"dubbo://provider1.example.com:20880/demo.Service?timestamp=200&revision=v1";
+        // Both providers have same address and revision but different 
timestamp values.
+        // After normalization (removing timestamp), they should produce the 
same cache key.
+
+        URL consumerURL = URL.valueOf(consumerUrl);
+        Collection<String> providers = new ArrayList<>();
+        providers.add(provider1);
+        providers.add(provider2);
+
+        // First call: build cache with both providers
+        List<URL> urls1 = registry.toUrlsWithoutEmpty(consumerURL, providers);
+        assertNotNull(urls1);
+        assertEquals(1, urls1.size());
+
+        // Get the normalized key that should be used
+        String normalizedKey1 = strip(provider1); // Should have timestamp 
removed
+        String normalizedKey2 = strip(provider2); // Should be same as 
normalizedKey1
+
+        assertEquals(normalizedKey1, normalizedKey2, "Normalized keys should 
be identical after removing timestamp");
+
+        // Verify the cached URL map uses normalized key
+        Map<String, ServiceAddressURL> stringUrls = 
registry.stringUrls.get(consumerURL);
+        assertNotNull(stringUrls);
+        assertTrue(stringUrls.containsKey(normalizedKey1), "Cache should use 
normalized key");
+        assertEquals(1, stringUrls.size(), "Should have exactly one cache 
entry for deduplicated provider");
+    }
+
+    private String strip(String rawProvider) throws Exception {
+        Method method = 
CacheableFailbackRegistry.class.getDeclaredMethod("stripOffVariableKeys", 
String.class);
+        method.setAccessible(true);
+        return (String) method.invoke(registry, rawProvider);
+    }
+
+    private static final class StubCacheableFailbackRegistry extends 
CacheableFailbackRegistry {
+        StubCacheableFailbackRegistry(URL url) {
+            super(url);
+        }
+
+        @Override
+        public void doRegister(URL url) {}
+
+        @Override
+        public void doUnregister(URL url) {}
+
+        @Override
+        public void doSubscribe(URL url, NotifyListener listener) {}
+
+        @Override
+        protected boolean isMatch(URL subscribeUrl, URL providerUrl) {
+            return true;
+        }
+
+        @Override
+        public boolean isAvailable() {
+            return false;
+        }
+    }
+}

Reply via email to