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;
+ }
+ }
+}