You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by al...@apache.org on 2021/11/04 09:10:19 UTC

[dubbo] branch 3.0 updated: Catch deploy error, improve module start waiting (#9204)

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

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


The following commit(s) were added to refs/heads/3.0 by this push:
     new c14347c  Catch deploy error, improve module start waiting (#9204)
c14347c is described below

commit c14347cbbe03d9b0cb5d407895e2a648f0be4dd7
Author: Gong Dewei <ky...@qq.com>
AuthorDate: Thu Nov 4 17:07:53 2021 +0800

    Catch deploy error, improve module start waiting (#9204)
    
    * Catch deploy error, improve module start waiting
    
    * throw exception of module start
    
    * Ignore checking new module after start application
    
    * throw deploy exception
---
 .../dubbo/common/deploy/AbstractDeployer.java      |  12 +-
 .../org/apache/dubbo/common/deploy/Deployer.java   |   1 +
 .../config/deploy/DefaultApplicationDeployer.java  | 136 +++++++++++++--------
 .../dubbo/config/deploy/DefaultModuleDeployer.java |  73 +++++++----
 4 files changed, 142 insertions(+), 80 deletions(-)

diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/deploy/AbstractDeployer.java b/dubbo-common/src/main/java/org/apache/dubbo/common/deploy/AbstractDeployer.java
index 921759f..ba53201 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/deploy/AbstractDeployer.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/deploy/AbstractDeployer.java
@@ -37,6 +37,8 @@ public abstract class AbstractDeployer<E extends ScopeModel> implements Deployer
 
     private volatile DeployState state = PENDING;
 
+    private volatile Throwable lastError;
+
     protected AtomicBoolean initialized = new AtomicBoolean(false);
 
     private List<DeployListener<E>> listeners = new ArrayList<>();
@@ -144,17 +146,23 @@ public abstract class AbstractDeployer<E extends ScopeModel> implements Deployer
         }
     }
 
-    protected void setFailed(Throwable cause) {
+    protected void setFailed(Throwable error) {
         this.state = FAILED;
+        this.lastError = error;
         for (DeployListener<E> listener : listeners) {
             try {
-                listener.onFailure(scopeModel, cause);
+                listener.onFailure(scopeModel, error);
             } catch (Throwable e) {
                 logger.error(getIdentifier() + " an exception occurred when handle failed event", e);
             }
         }
     }
 
+    @Override
+    public Throwable getError() {
+        return lastError;
+    }
+
     public boolean isInitialized() {
         return initialized.get();
     }
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/deploy/Deployer.java b/dubbo-common/src/main/java/org/apache/dubbo/common/deploy/Deployer.java
index 568dda2..da3eef9 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/deploy/Deployer.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/deploy/Deployer.java
@@ -89,4 +89,5 @@ public interface Deployer<E extends ScopeModel> {
 
     void removeDeployListener(DeployListener<E> listener);
 
+    Throwable getError();
 }
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java
index 69d8baf..5c89f57 100644
--- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java
+++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java
@@ -526,31 +526,36 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
                 throw new IllegalStateException(getIdentifier() + " is stopping or stopped, can not start again");
             }
 
-            // maybe call start again after add new module, check if any new module
-            boolean hasPendingModule = hasPendingModule();
-
-            if (isStarting()) {
-                // currently is starting, maybe both start by module and application
-                // if has new modules, start them
-                if (hasPendingModule) {
-                    startModules();
+            try {
+                // maybe call start again after add new module, check if any new module
+                boolean hasPendingModule = hasPendingModule();
+
+                if (isStarting()) {
+                    // currently is starting, maybe both start by module and application
+                    // if has new modules, start them
+                    if (hasPendingModule) {
+                        startModules();
+                    }
+                    // if is starting, reuse previous startFuture
+                    return startFuture;
                 }
-                // if is starting, reuse previous startFuture
-                return startFuture;
-            }
 
-            // if is started and no new module, just return
-            if (isStarted() && !hasPendingModule) {
-                return CompletableFuture.completedFuture(false);
-            }
+                // if is started and no new module, just return
+                if (isStarted() && !hasPendingModule) {
+                    return CompletableFuture.completedFuture(false);
+                }
 
-            // pending -> starting : first start app
-            // started -> starting : re-start app
-            onStarting();
+                // pending -> starting : first start app
+                // started -> starting : re-start app
+                onStarting();
 
-            initialize();
+                initialize();
 
-            doStart();
+                doStart();
+            } catch (Throwable e) {
+                onFailed(getIdentifier() + " start failure", e);
+                throw e;
+            }
 
             return startFuture;
         }
@@ -578,37 +583,40 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
         // prepare application instance
         prepareApplicationInstance();
 
-        executorRepository.getSharedExecutor().submit(() -> {
-            try {
-                while (isStarting()) {
-                    // notify when any module state changed
-                    synchronized (stateLock) {
-                        try {
-                            stateLock.wait(500);
-                        } catch (InterruptedException e) {
-                            // ignore
-                        }
-                    }
-
-                    // if has new module, do start again
-                    if (hasPendingModule()) {
-                        startModules();
-                    }
-                }
-            } catch (Exception e) {
-                logger.warn("waiting for application startup occurred an exception", e);
-            }
-        });
+        // Ignore checking new module after start
+//        executorRepository.getSharedExecutor().submit(() -> {
+//            try {
+//                while (isStarting()) {
+//                    // notify when any module state changed
+//                    synchronized (stateLock) {
+//                        try {
+//                            stateLock.wait(500);
+//                        } catch (InterruptedException e) {
+//                            // ignore
+//                        }
+//                    }
+//
+//                    // if has new module, do start again
+//                    if (hasPendingModule()) {
+//                        startModules();
+//                    }
+//                }
+//            } catch (Throwable e) {
+//                onFailed(getIdentifier() + " check start occurred an exception", e);
+//            }
+//        });
     }
 
     private void startModules() {
         // ensure init and start internal module first
         prepareInternalModule();
 
-        // filter and start pending modules, ignore new module during starting
-        applicationModel.getModuleModels().stream()
-            .filter(moduleModel -> moduleModel.getDeployer().isPending())
-            .forEach(moduleModel -> moduleModel.getDeployer().start());
+        // filter and start pending modules, ignore new module during starting, throw exception of module start
+        for (ModuleModel moduleModel : new ArrayList<>(applicationModel.getModuleModels())) {
+            if (moduleModel.getDeployer().isPending()) {
+                moduleModel.getDeployer().start();
+            }
+        }
     }
 
     @Override
@@ -848,7 +856,7 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
 
     @Override
     public void preDestroy() {
-        synchronized(destroyLock) {
+        synchronized (destroyLock) {
             if (isStopping() || isStopped()) {
                 return;
             }
@@ -865,7 +873,7 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
 
     @Override
     public void postDestroy() {
-        synchronized(destroyLock) {
+        synchronized (destroyLock) {
             // expect application model is destroyed before here
             if (isStopped()) {
                 return;
@@ -887,8 +895,8 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
 
                 onStopped();
             } catch (Throwable ex) {
-                logger.error(getIdentifier() + " an error occurred while stopping application: " + ex.getMessage(), ex);
-                setFailed(ex);
+                String msg = getIdentifier() + " an error occurred while stopping application: " + ex.getMessage();
+                onFailed(msg, ex);
             }
         }
     }
@@ -925,6 +933,19 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
                 case STOPPED:
                     onStopped();
                     break;
+                case FAILED:
+                    Throwable error = null;
+                    ModuleModel errorModule = null;
+                    for (ModuleModel moduleModel : applicationModel.getModuleModels()) {
+                        ModuleDeployer deployer = moduleModel.getDeployer();
+                        if (deployer.isFailed() && deployer.getError() != null) {
+                            error = deployer.getError();
+                            errorModule = moduleModel;
+                            break;
+                        }
+                    }
+                    onFailed(getIdentifier() + " found failed module: " + errorModule.getDesc(), error);
+                    break;
                 case PENDING:
                     // cannot change to pending from other state
                     // setPending();
@@ -935,7 +956,7 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
 
     private DeployState calculateState() {
         DeployState newState = DeployState.UNKNOWN;
-        int pending = 0, starting = 0, started = 0, stopping = 0, stopped = 0;
+        int pending = 0, starting = 0, started = 0, stopping = 0, stopped = 0, failed = 0;
         for (ModuleModel moduleModel : applicationModel.getModuleModels()) {
             ModuleDeployer deployer = moduleModel.getDeployer();
             if (deployer.isPending()) {
@@ -948,10 +969,14 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
                 stopping++;
             } else if (deployer.isStopped()) {
                 stopped++;
+            } else if (deployer.isFailed()) {
+                failed++;
             }
         }
 
-        if (started > 0) {
+        if (failed > 0) {
+            newState = DeployState.FAILED;
+        } else if (started > 0) {
             if (pending + starting + stopping + stopped == 0) {
                 // all modules have been started
                 newState = DeployState.STARTED;
@@ -1053,6 +1078,15 @@ public class DefaultApplicationDeployer extends AbstractDeployer<ApplicationMode
         }
     }
 
+    private void onFailed(String msg, Throwable ex) {
+        try {
+            setFailed(ex);
+            logger.error(msg, ex);
+        } finally {
+            completeStartFuture(false);
+        }
+    }
+
     private void destroyExecutorRepository() {
         // shutdown export/refer executor
         executorRepository.shutdownServiceExportExecutor();
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultModuleDeployer.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultModuleDeployer.java
index 14345eb..173b343 100644
--- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultModuleDeployer.java
+++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultModuleDeployer.java
@@ -127,41 +127,50 @@ public class DefaultModuleDeployer extends AbstractDeployer<ModuleModel> impleme
             throw new IllegalStateException(getIdentifier() + " is stopping or stopped, can not start again");
         }
 
-        if (isStarting() || isStarted()) {
-            return startFuture;
-        }
+        try {
+            if (isStarting() || isStarted()) {
+                return startFuture;
+            }
 
-        onModuleStarting();
+            onModuleStarting();
 
-        // initialize
-        applicationDeployer.initialize();
-        initialize();
+            // initialize
+            applicationDeployer.initialize();
+            initialize();
 
-        // export services
-        exportServices();
+            // export services
+            exportServices();
 
-        // prepare application instance
-        // exclude internal module to avoid wait itself
-        if (moduleModel != moduleModel.getApplicationModel().getInternalModule()) {
-            applicationDeployer.prepareApplicationInstance();
-        }
+            // prepare application instance
+            // exclude internal module to avoid wait itself
+            if (moduleModel != moduleModel.getApplicationModel().getInternalModule()) {
+                applicationDeployer.prepareApplicationInstance();
+            }
 
-        // refer services
-        referServices();
+            // refer services
+            referServices();
 
-        executorRepository.getSharedExecutor().submit(() -> {
-            try {
-                // wait for export finish
-                waitExportFinish();
-                // wait for refer finish
-                waitReferFinish();
-            } catch (Throwable e) {
-                logger.warn("wait for export/refer services occurred an exception", e);
-            } finally {
+            // if no async export/refer services, just set started
+            if (asyncExportingFutures.isEmpty() && asyncReferringFutures.isEmpty()) {
                 onModuleStarted();
+            } else {
+                executorRepository.getSharedExecutor().submit(() -> {
+                    try {
+                        // wait for export finish
+                        waitExportFinish();
+                        // wait for refer finish
+                        waitReferFinish();
+                    } catch (Throwable e) {
+                        logger.warn("wait for export/refer services occurred an exception", e);
+                    } finally {
+                        onModuleStarted();
+                    }
+                });
             }
-        });
-
+        } catch (Throwable e) {
+            onModuleFailed(getIdentifier() + " start failed: " + e.toString(), e);
+            throw e;
+        }
         return startFuture;
     }
 
@@ -248,6 +257,16 @@ public class DefaultModuleDeployer extends AbstractDeployer<ModuleModel> impleme
         }
     }
 
+    private void onModuleFailed(String msg, Throwable ex) {
+        try {
+            setFailed(ex);
+            logger.error(msg, ex);
+            applicationDeployer.notifyModuleChanged(moduleModel, DeployState.STARTED);
+        } finally {
+            completeStartFuture(false);
+        }
+    }
+
     private void completeStartFuture(boolean value) {
         if (startFuture != null && !startFuture.isDone()) {
             startFuture.complete(value);