You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by GitBox <gi...@apache.org> on 2020/08/20 17:00:04 UTC

[GitHub] [unomi] liatiusim opened a new pull request #186: Stop unomi when reactor error

liatiusim opened a new pull request #186:
URL: https://github.com/apache/unomi/pull/186


   In case elastic reactor is stopped, unomi starts an unhealthy loop, which requires crashing the app.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] sergehuber commented on pull request #186: Stop unomi when reactor error

Posted by GitBox <gi...@apache.org>.
sergehuber commented on pull request #186:
URL: https://github.com/apache/unomi/pull/186#issuecomment-677798193


   Hello, thank you for the PR, I put a comment in the code above.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] sergehuber commented on a change in pull request #186: Stop unomi when reactor error

Posted by GitBox <gi...@apache.org>.
sergehuber commented on a change in pull request #186:
URL: https://github.com/apache/unomi/pull/186#discussion_r474154325



##########
File path: persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java
##########
@@ -2081,13 +2081,25 @@ public T executeInClassLoader(Object... args) throws Exception {
         public T catchingExecuteInClassLoader(boolean logError, Object... args) {
             try {
                 return executeInClassLoader(timerName, args);
-            } catch (Throwable t) {
-                if (logError) {
-                    logger.error("Error while executing in class loader", t);
+            } catch (IllegalStateException e) {
+                // Reactor stopped - recovery is not possible
+                if (e.getMessage().contains("I/O reactor status: STOPPED")) {
+                    logger.error("I/O Reactor stopped - stopping application");
+                    System.exit(-1);

Review comment:
       System.exit is really not recommended in an OSGi environment. It would be better to use the UnomiManagementService.stopUnomi method.
   
   Ideally we would add a way for the ElasticSearch persistence implementation to recover from a failing connection but that's a lot more involved in terms of work.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] sergehuber commented on pull request #186: Stop unomi when reactor error

Posted by GitBox <gi...@apache.org>.
sergehuber commented on pull request #186:
URL: https://github.com/apache/unomi/pull/186#issuecomment-696589004


   Also please create a JIRA ticket for this PR and add it to the title of the PR. 
   
   Thank you.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] sergehuber commented on pull request #186: Stop unomi when reactor error

Posted by GitBox <gi...@apache.org>.
sergehuber commented on pull request #186:
URL: https://github.com/apache/unomi/pull/186#issuecomment-683721665


   @liatiusim have you seen my comments on this PR ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] sergehuber commented on pull request #186: Stop unomi when reactor error

Posted by GitBox <gi...@apache.org>.
sergehuber commented on pull request #186:
URL: https://github.com/apache/unomi/pull/186#issuecomment-678304504


   Here's an even better solution if you absolutely want to shut everything down :
   https://stackoverflow.com/questions/1916432/best-way-to-shutdown-an-osgi-container-specifically-equinox#:~:text=exit(0)%20to%20shut%20down,e.g.%20to%20free%20resources%20etc).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] liatiusim commented on a change in pull request #186: Stop unomi when reactor error

Posted by GitBox <gi...@apache.org>.
liatiusim commented on a change in pull request #186:
URL: https://github.com/apache/unomi/pull/186#discussion_r480117924



##########
File path: persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java
##########
@@ -2081,13 +2081,25 @@ public T executeInClassLoader(Object... args) throws Exception {
         public T catchingExecuteInClassLoader(boolean logError, Object... args) {
             try {
                 return executeInClassLoader(timerName, args);
-            } catch (Throwable t) {
-                if (logError) {
-                    logger.error("Error while executing in class loader", t);
+            } catch (IllegalStateException e) {
+                // Reactor stopped - recovery is not possible
+                if (e.getMessage().contains("I/O reactor status: STOPPED")) {
+                    logger.error("I/O Reactor stopped - stopping application");
+                    System.exit(-1);

Review comment:
       Thank you for the code review. I'll fix that soon




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] liatiusim commented on pull request #186: Unomi - 387: Stop Unomi in case of an error which its cannot recovery from

Posted by GitBox <gi...@apache.org>.
liatiusim commented on pull request #186:
URL: https://github.com/apache/unomi/pull/186#issuecomment-698124711


   Hey Serge, sorry for the late response.
   I added a Jira ticket for this PR. As for your comment about using the bundleContext.close(), I fixed it (didn't pushed yet), but I think that since other threads are running while the system bundle context is closed, there are errors and the application is not gracefully shutting down. For example, I get this error - 
   ```
   org.ops4j.pax.web.pax-web-extender-war [org.ops4j.pax.web.extender.war.internal.WebObserver] ERROR : Error scanning web bundle org.apache.unomi.wab [162]: null
   java.lang.NullPointerException
   ```
   and other errors as well, depends on the specific run.
   Probably because other threads are running and the bundles were already closed.
   Do you have any suggestions how to handle that?
   Thanks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] sergehuber commented on pull request #186: Stop unomi when reactor error

Posted by GitBox <gi...@apache.org>.
sergehuber commented on pull request #186:
URL: https://github.com/apache/unomi/pull/186#issuecomment-696589004


   Also please create a JIRA ticket for this PR and add it to the title of the PR. 
   
   Thank you.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] liatiusim commented on pull request #186: Unomi - 387: Stop Unomi in case of an error which its cannot recovery from

Posted by GitBox <gi...@apache.org>.
liatiusim commented on pull request #186:
URL: https://github.com/apache/unomi/pull/186#issuecomment-698124711


   Hey Serge, sorry for the late response.
   I added a Jira ticket for this PR. As for your comment about using the bundleContext.close(), I fixed it (didn't pushed yet), but I think that since other threads are running while the system bundle context is closed, there are errors and the application is not gracefully shutting down. For example, I get this error - 
   ```
   org.ops4j.pax.web.pax-web-extender-war [org.ops4j.pax.web.extender.war.internal.WebObserver] ERROR : Error scanning web bundle org.apache.unomi.wab [162]: null
   java.lang.NullPointerException
   ```
   and other errors as well, depends on the specific run.
   Probably because other threads are running and the bundles were already closed.
   Do you have any suggestions how to handle that?
   Thanks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [unomi] sergehuber merged pull request #186: Unomi - 387: Stop Unomi in case of an error which its cannot recovery from

Posted by GitBox <gi...@apache.org>.
sergehuber merged pull request #186:
URL: https://github.com/apache/unomi/pull/186


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org