This is an automated email from the ASF dual-hosted git repository.
albumenj pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/3.1 by this push:
new 213944c03e Add code that shows error code 2-2, 1-9, 5-1 with code
optimization. (#10452)
213944c03e is described below
commit 213944c03ecfd27b7a92257b92acde503cc3c628
Author: Andy Cheung <[email protected]>
AuthorDate: Sun Aug 14 10:40:44 2022 +0800
Add code that shows error code 2-2, 1-9, 5-1 with code optimization.
(#10452)
* Optimize javadoc in Protocol.
* Optimize javadoc in Protocol.
* Add error 2-2 in ReferenceConfig.
* Add error code 5-1 in CuratorZookeeperClient and Curator5ZookeeperClient.
* Optimize Javadoc of ZookeeperClient.
* Forbids the instantiation and optimize comments of UrlUtils.
* Fix typo in FrameworkExecutorRepository.
* Throw created illegalStateException instead of throwing a new one.
* Add error code 1-9 constructor of AbstractRegistry.
* Optimize comments and modifiers.
* Optimize appearance of error code 1-9.
---
.../manager/FrameworkExecutorRepository.java | 2 +-
.../org/apache/dubbo/common/utils/UrlUtils.java | 48 +++++++++++++++----
.../org/apache/dubbo/config/ReferenceConfig.java | 8 +++-
.../ServiceInstanceHostPortCustomizer.java | 2 +-
.../dubbo/registry/support/AbstractRegistry.java | 54 ++++++++++++++++++----
.../support/CacheableFailbackRegistry.java | 38 +++++++++++++--
.../registry/CacheableFailbackRegistryTest.java | 14 +++---
.../dubbo/remoting/zookeeper/ZookeeperClient.java | 20 ++++++--
.../curator5/Curator5ZookeeperClient.java | 14 ++++--
.../zookeeper/curator/CuratorZookeeperClient.java | 14 ++++--
.../main/java/org/apache/dubbo/rpc/Protocol.java | 30 +++++++++++-
11 files changed, 197 insertions(+), 47 deletions(-)
diff --git
a/dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/manager/FrameworkExecutorRepository.java
b/dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/manager/FrameworkExecutorRepository.java
index df4e10eb2a..d630ee473e 100644
---
a/dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/manager/FrameworkExecutorRepository.java
+++
b/dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/manager/FrameworkExecutorRepository.java
@@ -91,7 +91,7 @@ public class FrameworkExecutorRepository implements
Disposable {
/**
* Returns a scheduler from the scheduler list, call this method whenever
you need a scheduler for a cron job.
- * If your cron cannot burden the possible schedule delay caused by
sharing the same scheduler, please consider define a dedicate one.
+ * If your cron cannot burden the possible schedule delay caused by
sharing the same scheduler, please consider define a dedicated one.
*
* @return ScheduledExecutorService
*/
diff --git
a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java
b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java
index f0246c0192..11b1e3faa2 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java
@@ -63,10 +63,17 @@ import static
org.apache.dubbo.common.constants.RegistryConstants.SERVICE_REGIST
public class UrlUtils {
+ /**
+ * Forbids the instantiation.
+ */
+ private UrlUtils() {
+ throw new UnsupportedOperationException("No instance of 'UrlUtils' for
you! ");
+ }
+
/**
* in the url string,mark the param begin
*/
- private final static String URL_PARAM_STARTING_SYMBOL = "?";
+ private static final String URL_PARAM_STARTING_SYMBOL = "?";
public static URL parseURL(String address, Map<String, String> defaults) {
if (StringUtils.isEmpty(address)) {
@@ -212,7 +219,8 @@ public class UrlUtils {
}
public static Map<String, String> convertSubscribe(Map<String, String>
subscribe) {
- Map<String, String> newSubscribe = new HashMap<String, String>();
+ Map<String, String> newSubscribe = new HashMap<>();
+
for (Map.Entry<String, String> entry : subscribe.entrySet()) {
String serviceName = entry.getKey();
String serviceQuery = entry.getValue();
@@ -234,11 +242,13 @@ public class UrlUtils {
newSubscribe.put(serviceName, serviceQuery);
}
}
+
return newSubscribe;
}
public static Map<String, Map<String, String>> revertRegister(Map<String,
Map<String, String>> register) {
- Map<String, Map<String, String>> newRegister = new HashMap<String,
Map<String, String>>();
+ Map<String, Map<String, String>> newRegister = new HashMap<>();
+
for (Map.Entry<String, Map<String, String>> entry :
register.entrySet()) {
String serviceName = entry.getKey();
Map<String, String> serviceUrls = entry.getValue();
@@ -265,11 +275,14 @@ public class UrlUtils {
newRegister.put(serviceName, serviceUrls);
}
}
+
return newRegister;
}
public static Map<String, String> revertSubscribe(Map<String, String>
subscribe) {
- Map<String, String> newSubscribe = new HashMap<String, String>();
+
+ Map<String, String> newSubscribe = new HashMap<>();
+
for (Map.Entry<String, String> entry : subscribe.entrySet()) {
String serviceName = entry.getKey();
String serviceQuery = entry.getValue();
@@ -385,32 +398,47 @@ public class UrlUtils {
public static boolean isMatch(URL consumerUrl, URL providerUrl) {
String consumerInterface = consumerUrl.getServiceInterface();
String providerInterface = providerUrl.getServiceInterface();
- //FIXME accept providerUrl with '*' as interface name, after carefully
thought about all possible scenarios I think it's ok to add this condition.
+
+ // FIXME accept providerUrl with '*' as interface name, after
carefully thought about all possible scenarios I think it's ok to add this
condition.
+
+ // Return false if the consumer interface is not equals the provider
interface,
+ // except one of the interface configurations is equals '*' (i.e. any
value).
if (!(ANY_VALUE.equals(consumerInterface)
|| ANY_VALUE.equals(providerInterface)
|| StringUtils.isEquals(consumerInterface, providerInterface))) {
return false;
}
- if (!isMatchCategory(providerUrl.getCategory(DEFAULT_CATEGORY),
- consumerUrl.getCategory(DEFAULT_CATEGORY))) {
+ // If the category of provider URL does not match the category of
consumer URL.
+ // Usually, the provider URL's category is empty, and the default
category ('providers') is present.
+ // Hence, the category of the provider URL is 'providers'.
+ // Through observing of the Zookeeper registry, I found that the
category of the consumer URL is 'consumers'.
+ if (!isMatchCategory(providerUrl.getCategory(DEFAULT_CATEGORY),
consumerUrl.getCategory(DEFAULT_CATEGORY))) {
return false;
}
+
+ // If the provider is not enabled, return false.
if (!providerUrl.getParameter(ENABLED_KEY, true)
&& !ANY_VALUE.equals(consumerUrl.getParameter(ENABLED_KEY))) {
return false;
}
+ // Obtain consumer's group, version and classifier.
String consumerGroup = consumerUrl.getGroup();
String consumerVersion = consumerUrl.getVersion();
String consumerClassifier = consumerUrl.getParameter(CLASSIFIER_KEY,
ANY_VALUE);
+ // Obtain provider's group, version and classifier.
String providerGroup = providerUrl.getGroup();
String providerVersion = providerUrl.getVersion();
String providerClassifier = providerUrl.getParameter(CLASSIFIER_KEY,
ANY_VALUE);
- return (ANY_VALUE.equals(consumerGroup) ||
StringUtils.isEquals(consumerGroup, providerGroup) ||
StringUtils.isContains(consumerGroup, providerGroup))
- && (ANY_VALUE.equals(consumerVersion) ||
StringUtils.isEquals(consumerVersion, providerVersion))
- && (consumerClassifier == null ||
ANY_VALUE.equals(consumerClassifier) ||
StringUtils.isEquals(consumerClassifier, providerClassifier));
+
+ // If Group, Version, Classifier all matches, return true.
+ boolean groupMatches = ANY_VALUE.equals(consumerGroup) ||
StringUtils.isEquals(consumerGroup, providerGroup) ||
StringUtils.isContains(consumerGroup, providerGroup);
+ boolean versionMatches = ANY_VALUE.equals(consumerVersion) ||
StringUtils.isEquals(consumerVersion, providerVersion);
+ boolean classifierMatches = consumerClassifier == null ||
ANY_VALUE.equals(consumerClassifier) ||
StringUtils.isEquals(consumerClassifier, providerClassifier);
+
+ return groupMatches && versionMatches && classifierMatches;
}
public static boolean isMatchGlobPattern(String pattern, String value, URL
param) {
diff --git
a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
index 19d278329f..8dd76c073a 100644
---
a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
+++
b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
@@ -638,7 +638,9 @@ public class ReferenceConfig<T> extends
ReferenceConfigBase<T> {
private void checkInvokerAvailable() throws IllegalStateException {
if (shouldCheck() && !invoker.isAvailable()) {
- throw new IllegalStateException("Failed to check the status of the
service "
+ // 2-2 - No provider available.
+
+ IllegalStateException illegalStateException = new
IllegalStateException("Failed to check the status of the service "
+ interfaceName
+ ". No provider available for the service "
+ (group == null ? "" : group + "/")
@@ -648,6 +650,10 @@ public class ReferenceConfig<T> extends
ReferenceConfigBase<T> {
+ invoker.getUrl()
+ " to the consumer "
+ NetUtils.getLocalHost() + " use dubbo version " +
Version.getVersion());
+
+ logger.error("2-2", "provider not started", "", "No provider
available.", illegalStateException);
+
+ throw illegalStateException;
}
}
diff --git
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/metadata/ServiceInstanceHostPortCustomizer.java
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/metadata/ServiceInstanceHostPortCustomizer.java
index 3b8c193c3f..9e5a20742a 100644
---
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/metadata/ServiceInstanceHostPortCustomizer.java
+++
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/metadata/ServiceInstanceHostPortCustomizer.java
@@ -75,7 +75,7 @@ public class ServiceInstanceHostPortCustomizer implements
ServiceInstanceCustomi
// 4-2 - Can't find an instance URL using the default
preferredProtocol.
logger.warn("4-2", "typo in preferred protocol", "",
- "Can't find an instance URL using the default
preferredProtocol \"" + preferredProtocol + "\", " +
+ "Can't find an instance URL using the default
preferredProtocol \"" + preferredProtocol + "\", " +
"falling back to the strategy that pick the first
found protocol. " +
"Please try modifying the config of
dubbo.application.protocol");
diff --git
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java
index 170a684f78..b9e83baf79 100644
---
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java
+++
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java
@@ -69,7 +69,10 @@ import static
org.apache.dubbo.registry.Constants.REGISTRY_FILESAVE_SYNC_KEY;
import static org.apache.dubbo.registry.Constants.USER_HOME;
/**
- * AbstractRegistry. (SPI, Prototype, ThreadSafe)
+ * <p>
+ * Provides a fail-safe registry service backed by cache file. The
consumer/provider can still find each other when registry center crashed.
+ *
+ * (SPI, Prototype, ThreadSafe)
*/
public abstract class AbstractRegistry implements Registry {
@@ -101,7 +104,9 @@ public abstract class AbstractRegistry implements Registry {
protected RegistryManager registryManager;
protected ApplicationModel applicationModel;
- public AbstractRegistry(URL url) {
+ private static final String CAUSE_MULTI_DUBBO_USING_SAME_FILE = "multiple
Dubbo instance are using the same file";
+
+ protected AbstractRegistry(URL url) {
setUrl(url);
registryManager =
url.getOrDefaultApplicationModel().getBeanFactory().getBean(RegistryManager.class);
localCacheEnabled =
url.getParameter(REGISTRY_LOCAL_FILE_CACHE_ENABLED, true);
@@ -110,19 +115,36 @@ public abstract class AbstractRegistry implements
Registry {
if (localCacheEnabled) {
// Start file save timer
syncSaveFile = url.getParameter(REGISTRY_FILESAVE_SYNC_KEY, false);
+
String defaultFilename = System.getProperty(USER_HOME) +
DUBBO_REGISTRY + url.getApplication() +
"-" + url.getAddress().replaceAll(":", "-") + CACHE;
+
String filename = url.getParameter(FILE_KEY, defaultFilename);
File file = null;
+
if (ConfigUtils.isNotEmpty(filename)) {
file = new File(filename);
if (!file.exists() && file.getParentFile() != null &&
!file.getParentFile().exists()) {
if (!file.getParentFile().mkdirs()) {
- throw new IllegalArgumentException("Invalid registry
cache file " + file + ", cause: Failed to create directory " +
file.getParentFile() + "!");
+
+ IllegalArgumentException illegalArgumentException =
new IllegalArgumentException(
+ "Invalid registry cache file " + file + ", cause:
Failed to create directory " + file.getParentFile() + "!");
+
+ if (logger != null) {
+ // 1-9 failed to read / save registry cache file.
+
+ logger.error("1-9", "cache directory inaccessible",
+ "Try adjusting permission of the directory.",
+ "failed to create directory",
illegalArgumentException);
+ }
+
+ throw illegalArgumentException;
}
}
}
+
this.file = file;
+
// When starting the subscription center,
// we need to read the local cache file for future Registry fault
tolerance processing.
loadProperties();
@@ -189,13 +211,22 @@ public abstract class AbstractRegistry implements
Registry {
if (!lockfile.exists()) {
lockfile.createNewFile();
}
+
try (RandomAccessFile raf = new RandomAccessFile(lockfile, "rw");
FileChannel channel = raf.getChannel()) {
FileLock lock = channel.tryLock();
if (lock == null) {
- throw new IOException("Can not lock the registry cache
file " + file.getAbsolutePath() + ", " +
+
+ IOException ioException = new IOException("Can not lock
the registry cache file " + file.getAbsolutePath() + ", " +
"ignore and retry later, maybe multi java process use
the file, please config: dubbo.registry.file=xxx.properties");
+
+ // 1-9 failed to read / save registry cache file.
+ logger.warn("1-9", CAUSE_MULTI_DUBBO_USING_SAME_FILE, "",
+ "Adjust dubbo.registry.file.", ioException);
+
+ throw ioException;
}
+
// Save
try {
if (!file.exists()) {
@@ -226,26 +257,30 @@ public abstract class AbstractRegistry implements
Registry {
}
} catch (Throwable e) {
savePropertiesRetryTimes.incrementAndGet();
+
if (savePropertiesRetryTimes.get() >=
MAX_RETRY_TIMES_SAVE_PROPERTIES) {
if (e instanceof OverlappingFileLockException) {
// fix #9341, ignore OverlappingFileLockException
logger.info("Failed to save registry cache file for file
overlapping lock exception, file name " + file.getName());
} else {
// 1-9 failed to read / save registry cache file.
- logger.warn("1-9", "multiple Dubbo instance are using the
same file", "",
+ logger.warn("1-9", CAUSE_MULTI_DUBBO_USING_SAME_FILE, "",
"Failed to save registry cache file after retrying " +
MAX_RETRY_TIMES_SAVE_PROPERTIES + " times, cause: " + e.getMessage(), e);
}
+
savePropertiesRetryTimes.set(0);
return;
}
+
if (version < lastCacheChanged.get()) {
savePropertiesRetryTimes.set(0);
return;
} else {
registryCacheExecutor.execute(() ->
doSaveProperties(lastCacheChanged.incrementAndGet()));
}
+
if (!(e instanceof OverlappingFileLockException)) {
- logger.warn("1-9", "multiple Dubbo instance are using the same
file",
+ logger.warn("1-9", CAUSE_MULTI_DUBBO_USING_SAME_FILE,
"However, the retrying count limit is not exceeded. Dubbo
will still try.",
"Failed to save registry cache file, will retry, cause: "
+ e.getMessage(), e);
}
@@ -271,13 +306,13 @@ public abstract class AbstractRegistry implements
Registry {
}
} catch (IOException e) {
// 1-9 failed to read / save registry cache file.
- logger.warn("1-9", "multiple Dubbo instance are using the same
file", "",
+ logger.warn("1-9", CAUSE_MULTI_DUBBO_USING_SAME_FILE, "",
e.getMessage(), e);
} catch (Throwable e) {
// 1-9 failed to read / save registry cache file.
- logger.warn("1-9", "multiple Dubbo instance are using the same
file", "",
+ logger.warn("1-9", CAUSE_MULTI_DUBBO_USING_SAME_FILE, "",
"Failed to load registry cache file " + file, e);
}
}
@@ -442,7 +477,7 @@ public abstract class AbstractRegistry implements Registry {
}
/**
- * Notify changes from the Provider side.
+ * Notify changes from the provider side.
*
* @param url consumer side url
* @param listener listener
@@ -481,6 +516,7 @@ public abstract class AbstractRegistry implements Registry {
List<URL> categoryList = entry.getValue();
categoryNotified.put(category, categoryList);
listener.notify(categoryList);
+
// We will update our cache file after each notification.
// When our Registry has a subscribed failure due to network
jitter, we can return at least the existing cache URL.
if (localCacheEnabled) {
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 5a9fc8a439..b1cccf267e 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
@@ -62,7 +62,14 @@ import static
org.apache.dubbo.common.constants.RegistryConstants.ENABLE_EMPTY_P
import static
org.apache.dubbo.common.constants.RegistryConstants.PROVIDERS_CATEGORY;
/**
- * Useful for registries who's sdk returns raw string as provider instance,
for example, zookeeper and etcd.
+ * <p>
+ * Based on FailbackRegistry, it adds a URLAddress and URLParam cache to save
RAM space.
+ *
+ * <p>
+ * It's useful for registries whose sdk returns raw string as provider
instance. For example, Zookeeper and etcd.
+ *
+ * @see org.apache.dubbo.registry.support.FailbackRegistry
+ * @see org.apache.dubbo.registry.support.AbstractRegistry
*/
public abstract class CacheableFailbackRegistry extends FailbackRegistry {
private static final ErrorTypeAwareLogger logger =
LoggerFactory.getErrorTypeAwareLogger(CacheableFailbackRegistry.class);
@@ -71,16 +78,18 @@ public abstract class CacheableFailbackRegistry extends
FailbackRegistry {
protected Map<String, URLAddress> stringAddress = new
ConcurrentHashMap<>();
protected Map<String, URLParam> stringParam = new ConcurrentHashMap<>();
+
private ScheduledExecutorService cacheRemovalScheduler;
private int cacheRemovalTaskIntervalInMillis;
private int cacheClearWaitingThresholdInMillis;
+
private Map<ServiceAddressURL, Long> waitForRemove = new
ConcurrentHashMap<>();
private Semaphore semaphore = new Semaphore(1);
private final Map<String, String> extraParameters;
protected final Map<URL, Map<String, ServiceAddressURL>> stringUrls = new
ConcurrentHashMap<>();
- public CacheableFailbackRegistry(URL url) {
+ protected CacheableFailbackRegistry(URL url) {
super(url);
extraParameters = new HashMap<>(8);
extraParameters.put(CHECK_KEY, String.valueOf(false));
@@ -114,7 +123,7 @@ public abstract class CacheableFailbackRegistry extends
FailbackRegistry {
protected void evictURLCache(URL url) {
Map<String, ServiceAddressURL> oldURLs = stringUrls.remove(url);
try {
- if (oldURLs != null && oldURLs.size() > 0) {
+ if (oldURLs != null && !oldURLs.isEmpty()) {
logger.info("Evicting urls for service " + url.getServiceKey()
+ ", size " + oldURLs.size());
Long currentTimestamp = System.currentTimeMillis();
for (Map.Entry<String, ServiceAddressURL> entry :
oldURLs.entrySet()) {
@@ -127,8 +136,18 @@ public abstract class CacheableFailbackRegistry extends
FailbackRegistry {
}
}
} catch (Exception e) {
+ // It seems that the most possible statement that causes exception
is the 'schedule()' method.
+
+ // The executor that
FrameworkExecutorRepository.nextScheduledExecutor() method returns
+ // is made by Executors.newSingleThreadScheduledExecutor().
+
+ // After observing the code of
ScheduledThreadPoolExecutor.delayedExecute,
+ // it seems that it only throws RejectedExecutionException when
the thread pool is shutdown.
+
+ // When? FrameworkExecutorRepository gets destroyed.
+
// 1-3: URL evicting failed.
- logger.warn("1-3", "", "",
+ logger.warn("1-3", "thread pool getting destroyed", "",
"Failed to evict url for " + url.getServiceKey(), e);
}
}
@@ -216,11 +235,13 @@ public abstract class CacheableFailbackRegistry extends
FailbackRegistry {
protected ServiceAddressURL createURL(String rawProvider, URL consumerURL,
Map<String, String> extraParameters) {
boolean encoded = true;
+
// use encoded value directly to avoid URLDecoder.decode allocation.
int paramStartIdx = rawProvider.indexOf(ENCODED_QUESTION_MARK);
if (paramStartIdx == -1) {// if ENCODED_QUESTION_MARK does not show,
mark as not encoded.
encoded = false;
}
+
String[] parts = URLStrParser.parseRawURLToArrays(rawProvider,
paramStartIdx);
if (parts.length <= 1) {
// 1-5 Received URL without any parameters.
@@ -232,7 +253,10 @@ public abstract class CacheableFailbackRegistry extends
FailbackRegistry {
String rawAddress = parts[0];
String rawParams = parts[1];
+
+ // Workaround for 'effectively final': duplicate the encoded variable.
boolean isEncoded = encoded;
+
URLAddress address = stringAddress.computeIfAbsent(rawAddress, k ->
URLAddress.parse(k, getDefaultURLProtocol(), isEncoded));
address.setTimestamp(System.currentTimeMillis());
@@ -240,9 +264,11 @@ public abstract class CacheableFailbackRegistry extends
FailbackRegistry {
param.setTimestamp(System.currentTimeMillis());
ServiceAddressURL cachedURL = createServiceURL(address, param,
consumerURL);
+
if (isMatch(consumerURL, cachedURL)) {
return cachedURL;
}
+
return null;
}
@@ -332,7 +358,9 @@ public abstract class CacheableFailbackRegistry extends
FailbackRegistry {
protected abstract boolean isMatch(URL subscribeUrl, URL providerUrl);
-
+ /**
+ * The cached URL removal task, which will be run on a scheduled thread
pool. (It will be run after a delay.)
+ */
private class RemovalTask implements Runnable {
@Override
public void run() {
diff --git
a/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/CacheableFailbackRegistryTest.java
b/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/CacheableFailbackRegistryTest.java
index 0e52a74aa6..946e0eead1 100644
---
a/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/CacheableFailbackRegistryTest.java
+++
b/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/CacheableFailbackRegistryTest.java
@@ -34,7 +34,7 @@ import static
org.apache.dubbo.common.constants.RegistryConstants.EMPTY_PROTOCOL
import static
org.apache.dubbo.common.constants.RegistryConstants.ENABLE_EMPTY_PROTECTION_KEY;
import static org.junit.jupiter.api.Assertions.assertEquals;
-public class CacheableFailbackRegistryTest {
+class CacheableFailbackRegistryTest {
static String service;
static URL serviceUrl;
@@ -70,7 +70,7 @@ public class CacheableFailbackRegistryTest {
}
@Test
- public void testFullURLCache() {
+ void testFullURLCache() {
final AtomicReference<Integer> resCount = new AtomicReference<>(0);
registry = new MockCacheableRegistryImpl(registryUrl);
URL url = URLStrParser.parseEncodedStr(urlStr);
@@ -101,7 +101,7 @@ public class CacheableFailbackRegistryTest {
}
@Test
- public void testURLAddressCache() {
+ void testURLAddressCache() {
final AtomicReference<Integer> resCount = new AtomicReference<>(0);
registry = new MockCacheableRegistryImpl(registryUrl);
URL url = URLStrParser.parseEncodedStr(urlStr);
@@ -127,7 +127,7 @@ public class CacheableFailbackRegistryTest {
}
@Test
- public void testURLParamCache() {
+ void testURLParamCache() {
final AtomicReference<Integer> resCount = new AtomicReference<>(0);
registry = new MockCacheableRegistryImpl(registryUrl);
URL url = URLStrParser.parseEncodedStr(urlStr);
@@ -153,7 +153,7 @@ public class CacheableFailbackRegistryTest {
}
@Test
- public void testRemove() {
+ void testRemove() {
final AtomicReference<Integer> resCount = new AtomicReference<>(0);
registry = new MockCacheableRegistryImpl(registryUrl);
URL url = URLStrParser.parseEncodedStr(urlStr);
@@ -193,7 +193,7 @@ public class CacheableFailbackRegistryTest {
}
@Test
- public void testEmptyProtection() {
+ void testEmptyProtection() {
final AtomicReference<Integer> resCount = new AtomicReference<>(0);
final AtomicReference<List<URL>> currentUrls = new AtomicReference<>();
final List<URL> EMPTY_LIST = new ArrayList<>();
@@ -239,6 +239,4 @@ public class CacheableFailbackRegistryTest {
assertEquals(EMPTY_LIST, currentUrls.get());
}
-
-
}
diff --git
a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/zookeeper/ZookeeperClient.java
b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/zookeeper/ZookeeperClient.java
index dfdc403690..bb001da671 100644
---
a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/zookeeper/ZookeeperClient.java
+++
b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/zookeeper/ZookeeperClient.java
@@ -47,18 +47,28 @@ public interface ZookeeperClient {
List<String> addChildListener(String path, ChildListener listener);
/**
- * @param path directory. All child of path will be listened.
- * @param listener
+ * Attach data listener to current Zookeeper client.
+ *
+ * @param path directory. All children of path will be listened.
+ * @param listener The data listener object.
*/
void addDataListener(String path, DataListener listener);
/**
- * @param path directory. All child of path will be listened.
- * @param listener
- * @param executor another thread
+ * Attach data listener to current Zookeeper client. The listener will be
executed using the given executor.
+ *
+ * @param path directory. All children of path will be listened.
+ * @param listener The data listener object.
+ * @param executor the executor that will execute the listener.
*/
void addDataListener(String path, DataListener listener, Executor
executor);
+ /**
+ * Detach data listener.
+ *
+ * @param path directory. All listener of children of the path will be
detached.
+ * @param listener The data listener object.
+ */
void removeDataListener(String path, DataListener listener);
void removeChildListener(String path, ChildListener listener);
diff --git
a/dubbo-remoting/dubbo-remoting-zookeeper-curator5/src/main/java/org/apache/dubbo/remoting/zookeeper/curator5/Curator5ZookeeperClient.java
b/dubbo-remoting/dubbo-remoting-zookeeper-curator5/src/main/java/org/apache/dubbo/remoting/zookeeper/curator5/Curator5ZookeeperClient.java
index 52c1d9f301..604f29d38a 100644
---
a/dubbo-remoting/dubbo-remoting-zookeeper-curator5/src/main/java/org/apache/dubbo/remoting/zookeeper/curator5/Curator5ZookeeperClient.java
+++
b/dubbo-remoting/dubbo-remoting-zookeeper-curator5/src/main/java/org/apache/dubbo/remoting/zookeeper/curator5/Curator5ZookeeperClient.java
@@ -18,7 +18,7 @@ package org.apache.dubbo.remoting.zookeeper.curator5;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.config.configcenter.ConfigItem;
-import org.apache.dubbo.common.logger.Logger;
+import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.remoting.zookeeper.AbstractZookeeperClient;
import org.apache.dubbo.remoting.zookeeper.ChildListener;
@@ -59,7 +59,7 @@ import static
org.apache.dubbo.common.constants.CommonConstants.TIMEOUT_KEY;
public class Curator5ZookeeperClient extends
AbstractZookeeperClient<Curator5ZookeeperClient.NodeCacheListenerImpl,
Curator5ZookeeperClient.CuratorWatcherImpl> {
- protected static final Logger logger =
LoggerFactory.getLogger(Curator5ZookeeperClient.class);
+ protected static final ErrorTypeAwareLogger logger =
LoggerFactory.getErrorTypeAwareLogger(Curator5ZookeeperClient.class);
private static final Charset CHARSET = StandardCharsets.UTF_8;
private final CuratorFramework client;
@@ -94,9 +94,17 @@ public class Curator5ZookeeperClient extends
AbstractZookeeperClient<Curator5Zoo
client.getConnectionStateListenable().addListener(new
CuratorConnectionStateListener(url));
client.start();
boolean connected = client.blockUntilConnected(timeout,
TimeUnit.MILLISECONDS);
+
if (!connected) {
- throw new IllegalStateException("zookeeper not connected");
+ IllegalStateException illegalStateException = new
IllegalStateException("zookeeper not connected");
+
+ // 5-1 Failed to connect to configuration center.
+ logger.error("5-1", "Zookeeper server offline", "",
+ "Failed to connect with zookeeper", illegalStateException);
+
+ throw illegalStateException;
}
+
} catch (Exception e) {
throw new IllegalStateException(e.getMessage(), e);
}
diff --git
a/dubbo-remoting/dubbo-remoting-zookeeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java
b/dubbo-remoting/dubbo-remoting-zookeeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java
index 13a0018178..30cc4d574a 100644
---
a/dubbo-remoting/dubbo-remoting-zookeeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java
+++
b/dubbo-remoting/dubbo-remoting-zookeeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java
@@ -18,7 +18,7 @@ package org.apache.dubbo.remoting.zookeeper.curator;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.config.configcenter.ConfigItem;
-import org.apache.dubbo.common.logger.Logger;
+import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.utils.NamedThreadFactory;
import org.apache.dubbo.common.utils.StringUtils;
@@ -63,7 +63,7 @@ import static
org.apache.dubbo.common.constants.CommonConstants.TIMEOUT_KEY;
public class CuratorZookeeperClient extends
AbstractZookeeperClient<CuratorZookeeperClient.NodeCacheListenerImpl,
CuratorZookeeperClient.CuratorWatcherImpl> {
- protected static final Logger logger =
LoggerFactory.getLogger(CuratorZookeeperClient.class);
+ protected static final ErrorTypeAwareLogger logger =
LoggerFactory.getErrorTypeAwareLogger(CuratorZookeeperClient.class);
private static final Charset CHARSET = StandardCharsets.UTF_8;
private final CuratorFramework client;
@@ -97,10 +97,18 @@ public class CuratorZookeeperClient extends
AbstractZookeeperClient<CuratorZooke
client = builder.build();
client.getConnectionStateListenable().addListener(new
CuratorConnectionStateListener(url));
client.start();
+
boolean connected = client.blockUntilConnected(timeout,
TimeUnit.MILLISECONDS);
if (!connected) {
- throw new IllegalStateException("zookeeper not connected");
+ IllegalStateException illegalStateException = new
IllegalStateException("zookeeper not connected");
+
+ // 5-1 Failed to connect to configuration center.
+ logger.error("5-1", "Zookeeper server offline", "",
+ "Failed to connect with zookeeper", illegalStateException);
+
+ throw illegalStateException;
}
+
CuratorWatcherImpl.closed = false;
} catch (Exception e) {
close();
diff --git
a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/Protocol.java
b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/Protocol.java
index 838801dbfc..0bbc24f34c 100644
--- a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/Protocol.java
+++ b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/Protocol.java
@@ -25,7 +25,35 @@ import java.util.Collections;
import java.util.List;
/**
- * Protocol. (API/SPI, Singleton, ThreadSafe)
+ * RPC Protocol extension interface, which encapsulates the details of remote
invocation. <br /><br />
+ *
+ * <p>Conventions:
+ *
+ * <li>
+ * When user invokes the 'invoke()' method in object that the method
'refer()' returns,
+ * the protocol needs to execute the 'invoke()' method of Invoker object
that received by 'export()' method,
+ * which should have the same URL.
+ * </li>
+ *
+ * <li>
+ * Invoker that returned by 'refer()' is implemented by the protocol. The
remote invocation request should be sent by that Invoker.
+ * </li>
+ *
+ * <li>
+ * The invoker that 'export()' receives will be implemented by framework.
Protocol implementation should not care with that.
+ * </li>
+ *
+ * <p>Attentions:
+ *
+ * <li>
+ * The Protocol implementation does not care the transparent proxy. The
invoker will be converted to business interface by other layer.
+ * </li>
+ *
+ * <li>
+ * The protocol doesn't need to be backed by TCP connection. It can also
be backed by file sharing or inter-process communication.
+ * </li>
+ *
+ * (API/SPI, Singleton, ThreadSafe)
*/
@SPI(value = "dubbo", scope = ExtensionScope.FRAMEWORK)
public interface Protocol {
