Derby
  1. Derby
  2. DERBY-4304

Network Server shutdown should handle exceptions and finish the server shutdown completely

    Details

      Description

      While working on DERBY-4053, found that an exception from Connection.close was not handled properly by the server shutdown code which caused a new instance server startup to hang. Resolved the problem with Connection close but in general, we should
      1) Make sure an exception during shutdown processing does not prevent the remaining shutdown tasks, like closing the server socket from occurring.
      2) Make sure any exceptions that occur in shutdown processing are reported to the console.

      1. DERBY4304_fixNPE_patch2_diff.txt
        3 kB
        Mamta A. Satoor
      2. DERBY4304_handleExceptions_patch1_diff.txt
        5 kB
        Mamta A. Satoor
      3. DERBY4304ShutdownException_patch3_diff.txt
        10 kB
        Mamta A. Satoor
      4. logAfterPatch3Changes.txt
        54 kB
        Mamta A. Satoor
      5. logBeforePatch3Changes.txt
        48 kB
        Mamta A. Satoor

        Issue Links

          Activity

          Mamta A. Satoor created issue -
          Mamta A. Satoor made changes -
          Field Original Value New Value
          Link This issue is related to DERBY-4053 [ DERBY-4053 ]
          Tiago R. Espinha made changes -
          Link This issue relates to DERBY-4306 [ DERBY-4306 ]
          Hide
          Mamta A. Satoor added a comment -

          I have made changes to NetworkServerControlImpl.java so we handle the exceptions during different steps of server shutdown and continue with the next step in shutdown rather than bailing out after the first exception. I ran junit suite and didn't see any new failure except the known intermittent DERBY-4307 (RuntimeinfoTest Assertion failure). I will run derbyall next. If noone has any comments on the patch, I can check it in later this morning so it can be migrated to 10.5 codeline before we cut a release candidate there.

          Show
          Mamta A. Satoor added a comment - I have made changes to NetworkServerControlImpl.java so we handle the exceptions during different steps of server shutdown and continue with the next step in shutdown rather than bailing out after the first exception. I ran junit suite and didn't see any new failure except the known intermittent DERBY-4307 (RuntimeinfoTest Assertion failure). I will run derbyall next. If noone has any comments on the patch, I can check it in later this morning so it can be migrated to 10.5 codeline before we cut a release candidate there.
          Mamta A. Satoor made changes -
          Mamta A. Satoor made changes -
          Assignee Mamta A. Satoor [ mamtas ]
          Hide
          Knut Anders Hatlen added a comment -

          I noticed one small inconsistency: When iterating over sessionTable, the try block encloses the entire loop, so an error in the middle of the table will prevent the rest of the sessions from being closed. When iterating over threadList, the try block only encloses the body of the loop, so we'll continue shutting down the rest of the threads in case of an error. It would be good if the two loops were handled the same way.

          Many of the lines use an odd mix of both tabs and spaces on the same line.

          Show
          Knut Anders Hatlen added a comment - I noticed one small inconsistency: When iterating over sessionTable, the try block encloses the entire loop, so an error in the middle of the table will prevent the rest of the sessions from being closed. When iterating over threadList, the try block only encloses the body of the loop, so we'll continue shutting down the rest of the threads in case of an error. It would be good if the two loops were handled the same way. Many of the lines use an odd mix of both tabs and spaces on the same line.
          Hide
          Mamta A. Satoor added a comment -

          Knut, thanks for reviewing the patch. I will change the sessionTable loop to do try catch one session at a time rather than the entire loop. I meant to do it that way but obviously didn't. I will look at the tabs and spaces issue.

          Show
          Mamta A. Satoor added a comment - Knut, thanks for reviewing the patch. I will change the sessionTable loop to do try catch one session at a time rather than the entire loop. I meant to do it that way but obviously didn't. I will look at the tabs and spaces issue.
          Hide
          Mamta A. Satoor added a comment -

          I have committed the changes into trunk with revision 794303 and commit comments as follows. Working on mergine and running the tests on 10.5 codeline
          **********
          I have made changes to NetworkServerControlImpl.java so we handle the exceptions during different steps of server shutdown and continue with the next step in shutdown rather than bailing out after the first exception.
          **********

          Show
          Mamta A. Satoor added a comment - I have committed the changes into trunk with revision 794303 and commit comments as follows. Working on mergine and running the tests on 10.5 codeline ********** I have made changes to NetworkServerControlImpl.java so we handle the exceptions during different steps of server shutdown and continue with the next step in shutdown rather than bailing out after the first exception. **********
          Hide
          Kathey Marsden added a comment - - edited

          Thanks Mamta for making this change. I don't think it should go into to 10.5 until we have analyzed and perhaps resolved some of the new errors that will show up in the log and console. I think errors like the NullPointerException unregistering might be disconcerting.

          Show
          Kathey Marsden added a comment - - edited Thanks Mamta for making this change. I don't think it should go into to 10.5 until we have analyzed and perhaps resolved some of the new errors that will show up in the log and console. I think errors like the NullPointerException unregistering might be disconcerting.
          Hide
          Kathey Marsden added a comment -

          Mamta I noted that at least in the case of DERBY-4310, the exception stack trace does not print to the console, just the error message. This is the contents of serverConsole.log:
          2009-07-16 17:05:43.546 GMT : Apache Derby Network Server - 10.6.0.0 alpha - (794751) started and ready to accept connections on port 1527

          2009-07-16 17:05:45.078 GMT : Unexpected exception:
          Table/View 'APP.FOO' does not exist.

          Can you make it so the full stack trace prints as well. That would be much more helpful. To try it out, run jdbcapi.XATest and look at logs/serverConsoleOutput.logs.

          Show
          Kathey Marsden added a comment - Mamta I noted that at least in the case of DERBY-4310 , the exception stack trace does not print to the console, just the error message. This is the contents of serverConsole.log: 2009-07-16 17:05:43.546 GMT : Apache Derby Network Server - 10.6.0.0 alpha - (794751) started and ready to accept connections on port 1527 2009-07-16 17:05:45.078 GMT : Unexpected exception: Table/View 'APP.FOO' does not exist. Can you make it so the full stack trace prints as well. That would be much more helpful. To try it out, run jdbcapi.XATest and look at logs/serverConsoleOutput.logs.
          Kathey Marsden made changes -
          Link This issue is related to DERBY-4310 [ DERBY-4310 ]
          Hide
          Mamta A. Satoor added a comment -

          It seems that when running insane build, we do not put the stack trace in logs/serverConsoleOutput.log Kathey must be running insane. I will see what can we do in the code to have the stack trace printed no matter what kind of build we are working with.

          Show
          Mamta A. Satoor added a comment - It seems that when running insane build, we do not put the stack trace in logs/serverConsoleOutput.log Kathey must be running insane. I will see what can we do in the code to have the stack trace printed no matter what kind of build we are working with.
          Hide
          Mamta A. Satoor added a comment -

          with revision 796316, I have fixed the problem of stack trace printing. The commit comments were as follows
          **************
          DERBY-4304
          When running in insane mode, the stack trace of the exception was not getting printed in the console log file logs/serverConsoleOutput.log The reason for this is that the code was
          doing following
          consolePropertyMessage("DRDA_UnexpectedException.S",
          exception.getMessage());
          consoleExceptionPrintTrace(exception);
          That is, the stack trace printing was happening after the call to consolePropertyMessage. The call to consolePropertyMessage results into a call to consolePropertyMessageWork which may throw an exception back depending on the type of the error it is handling. If this method does throw an exception, then consoleExceptionPrintTrace does not get a chance to dump the stack trace into the logs/serverConsoleOutput. (Probably in sane mode, there is some other additional place where we do the stack trace printing and that is why we saw the stack trace despite consolePropertyMessage throwing an exception.) To fix the problem, I have switched the order of the 2 calls above and that makes sure that we do print the stack trace for debugging purpose.
          **************

          Show
          Mamta A. Satoor added a comment - with revision 796316, I have fixed the problem of stack trace printing. The commit comments were as follows ************** DERBY-4304 When running in insane mode, the stack trace of the exception was not getting printed in the console log file logs/serverConsoleOutput.log The reason for this is that the code was doing following consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); consoleExceptionPrintTrace(exception); That is, the stack trace printing was happening after the call to consolePropertyMessage. The call to consolePropertyMessage results into a call to consolePropertyMessageWork which may throw an exception back depending on the type of the error it is handling. If this method does throw an exception, then consoleExceptionPrintTrace does not get a chance to dump the stack trace into the logs/serverConsoleOutput. (Probably in sane mode, there is some other additional place where we do the stack trace printing and that is why we saw the stack trace despite consolePropertyMessage throwing an exception.) To fix the problem, I have switched the order of the 2 calls above and that makes sure that we do print the stack trace for debugging purpose. **************
          Hide
          Mamta A. Satoor added a comment -

          Kathey asked following on the derby-dev
          ****************
          Mamta, if consolePropertyMessage is throwing an exception, does that mean still we will not complete the other shutdown tasks?
          ****************

          I think that might be possible Maybe there is some other method which just does the job of printing stuff and not trying to handle it. I will take a look.

          Show
          Mamta A. Satoor added a comment - Kathey asked following on the derby-dev **************** Mamta, if consolePropertyMessage is throwing an exception, does that mean still we will not complete the other shutdown tasks? **************** I think that might be possible Maybe there is some other method which just does the job of printing stuff and not trying to handle it. I will take a look.
          Hide
          Mamta A. Satoor added a comment -

          I had finished running the junit suite before checking in and that ran fine with only one known failure DERBY-4307 testRunTests. derbyall had not finished before I checked in. It just finished and I see one NPE which may be same as DERBY-4306. I will investigate further into it. The NPE stack trace is as follows
          The failing test is DerbyNetAutoStart
          Starting test case 1.
          java.lang.NullPointerException
          at org.apache.derby.impl.services.jmx.JMXManagementService.unregisterMBean(JMXManagementService.java:286)
          at org.apache.derby.impl.services.jmx.JMXManagementService.unregisterMBean(JMXManagementService.java:277)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.blockingStart(NetworkServerControlImpl.java:892)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37)
          at java.lang.reflect.Method.invoke(Method.java:599)
          at org.apache.derby.iapi.jdbc.DRDAServerStarter.run(DRDAServerStarter.java:236)
          at java.lang.Thread.run(Thread.java:735)
          Starting test case 2.
          java.lang.NullPointerException
          at org.apache.derby.impl.services.jmx.JMXManagementService.unregisterMBean(JMXManagementService.java:286)
          at org.apache.derby.impl.services.jmx.JMXManagementService.unregisterMBean(JMXManagementService.java:277)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.blockingStart(NetworkServerControlImpl.java:892)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37)
          at java.lang.reflect.Method.invoke(Method.java:599)
          at org.apache.derby.iapi.jdbc.DRDAServerStarter.run(DRDAServerStarter.java:236)
          at java.lang.Thread.run(Thread.java:735)

          Show
          Mamta A. Satoor added a comment - I had finished running the junit suite before checking in and that ran fine with only one known failure DERBY-4307 testRunTests. derbyall had not finished before I checked in. It just finished and I see one NPE which may be same as DERBY-4306 . I will investigate further into it. The NPE stack trace is as follows The failing test is DerbyNetAutoStart Starting test case 1. java.lang.NullPointerException at org.apache.derby.impl.services.jmx.JMXManagementService.unregisterMBean(JMXManagementService.java:286) at org.apache.derby.impl.services.jmx.JMXManagementService.unregisterMBean(JMXManagementService.java:277) at org.apache.derby.impl.drda.NetworkServerControlImpl.blockingStart(NetworkServerControlImpl.java:892) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37) at java.lang.reflect.Method.invoke(Method.java:599) at org.apache.derby.iapi.jdbc.DRDAServerStarter.run(DRDAServerStarter.java:236) at java.lang.Thread.run(Thread.java:735) Starting test case 2. java.lang.NullPointerException at org.apache.derby.impl.services.jmx.JMXManagementService.unregisterMBean(JMXManagementService.java:286) at org.apache.derby.impl.services.jmx.JMXManagementService.unregisterMBean(JMXManagementService.java:277) at org.apache.derby.impl.drda.NetworkServerControlImpl.blockingStart(NetworkServerControlImpl.java:892) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37) at java.lang.reflect.Method.invoke(Method.java:599) at org.apache.derby.iapi.jdbc.DRDAServerStarter.run(DRDAServerStarter.java:236) at java.lang.Thread.run(Thread.java:735)
          Hide
          Mamta A. Satoor added a comment -

          Because of the NPE, I will backout my change for now until I figure out what is happening.

          Show
          Mamta A. Satoor added a comment - Because of the NPE, I will backout my change for now until I figure out what is happening.
          Hide
          Mamta A. Satoor added a comment -

          Backed out changes 796316 with revision 796372 until I can figure out the cause of NPE in DerbyNetAutoStart

          Show
          Mamta A. Satoor added a comment - Backed out changes 796316 with revision 796372 until I can figure out the cause of NPE in DerbyNetAutoStart
          Hide
          Mamta A. Satoor added a comment -

          Tiago noticed (
          [ https://issues.apache.org/jira/browse/DERBY-4306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12731644#action_12731644 ]) following NPE with my checkin 796316 (which has been rolled back)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.isMsgProperty(NetworkServerControlImpl.java:3460)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.localizeMessage(NetworkServerControlImpl.java:3397)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.consolePropertyMessageWork(NetworkServerControlImpl.java:3195)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.consolePropertyMessage(NetworkServerControlImpl.java:1888)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.blockingStart(NetworkServerControlImpl.java:895)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at java.lang.reflect.Method.invoke(Method.java:597)
          at org.apache.derby.iapi.jdbc.DRDAServerStarter.run(DRDAServerStarter.java:236)
          at java.lang.Thread.run(Thread.java:619)

          I debugged the NPE and found that my try catch block below was getting NPE from mgmtService.unregisterMBean(versionMBean);
          try

          { mgmtService.unregisterMBean(versionMBean); mgmtService.unregisterMBean(networkServerMBean); } catch (Exception exception) { consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); consoleExceptionPrintTrace(exception); }
          This NPE from mgmtService.unregisterMBean(versionMBean); is caught by the catch clause above. The problem though is that exception.getMessage() for this NPE returns null and our existing code for consolePropertyMessage is not written to handle a null 2nd parameter. But the work that is being done by consolePropertyMessage on exception.getMessage() is also done by the next line of code in try catch block which is consoleExceptionPrintTrace(exception); In fact, consoleExceptionPrintTrace handles the null exception.getMessage() fine. Because of that I am suggesting that I replace the call consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); with consolePropertyMessage("DRDA_UnexpectedException.S", true);

          So, the new try catch will look as follows
          try { mgmtService.unregisterMBean(versionMBean); mgmtService.unregisterMBean(networkServerMBean); }

          catch (Exception exception)

          { consolePropertyMessage("DRDA_UnexpectedException.S", true); consoleExceptionPrintTrace(exception); }

          In fact, I am changing all the try catches introduced for different steps of shutdown to look like above. I am running the junit tests right now. Once derbyall and junit finish, I will go ahead and commit these changes. Any feedback greatly appreciated.

          Show
          Mamta A. Satoor added a comment - Tiago noticed ( [ https://issues.apache.org/jira/browse/DERBY-4306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12731644#action_12731644 ]) following NPE with my checkin 796316 (which has been rolled back) at org.apache.derby.impl.drda.NetworkServerControlImpl.isMsgProperty(NetworkServerControlImpl.java:3460) at org.apache.derby.impl.drda.NetworkServerControlImpl.localizeMessage(NetworkServerControlImpl.java:3397) at org.apache.derby.impl.drda.NetworkServerControlImpl.consolePropertyMessageWork(NetworkServerControlImpl.java:3195) at org.apache.derby.impl.drda.NetworkServerControlImpl.consolePropertyMessage(NetworkServerControlImpl.java:1888) at org.apache.derby.impl.drda.NetworkServerControlImpl.blockingStart(NetworkServerControlImpl.java:895) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.apache.derby.iapi.jdbc.DRDAServerStarter.run(DRDAServerStarter.java:236) at java.lang.Thread.run(Thread.java:619) I debugged the NPE and found that my try catch block below was getting NPE from mgmtService.unregisterMBean(versionMBean); try { mgmtService.unregisterMBean(versionMBean); mgmtService.unregisterMBean(networkServerMBean); } catch (Exception exception) { consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); consoleExceptionPrintTrace(exception); } This NPE from mgmtService.unregisterMBean(versionMBean); is caught by the catch clause above. The problem though is that exception.getMessage() for this NPE returns null and our existing code for consolePropertyMessage is not written to handle a null 2nd parameter. But the work that is being done by consolePropertyMessage on exception.getMessage() is also done by the next line of code in try catch block which is consoleExceptionPrintTrace(exception); In fact, consoleExceptionPrintTrace handles the null exception.getMessage() fine. Because of that I am suggesting that I replace the call consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); with consolePropertyMessage("DRDA_UnexpectedException.S", true); So, the new try catch will look as follows try { mgmtService.unregisterMBean(versionMBean); mgmtService.unregisterMBean(networkServerMBean); } catch (Exception exception) { consolePropertyMessage("DRDA_UnexpectedException.S", true); consoleExceptionPrintTrace(exception); } In fact, I am changing all the try catches introduced for different steps of shutdown to look like above. I am running the junit tests right now. Once derbyall and junit finish, I will go ahead and commit these changes. Any feedback greatly appreciated.
          Hide
          Kathey Marsden added a comment -

          Mamta, will this also take care of the problem where printing out the exception throws a new Exception so the remaining shutdown tasks do not execute?

          If you post a patch before you commit, I will do a quick review.

          I wish we could somehow add tests for this issue, but because it exposes problems that shouldn't happen, as soon as we fix those we lose our test cases for this fix.

          Show
          Kathey Marsden added a comment - Mamta, will this also take care of the problem where printing out the exception throws a new Exception so the remaining shutdown tasks do not execute? If you post a patch before you commit, I will do a quick review. I wish we could somehow add tests for this issue, but because it exposes problems that shouldn't happen, as soon as we fix those we lose our test cases for this fix.
          Hide
          Mamta A. Satoor added a comment -

          I am attaching(DERBY4304_fixNPE_patch2_diff.txt) a simple fix to NPE problem. During one of the shutdown steps, DERBY-4306 was throwing a npe(we have fixed DERBY-4306 now) and the shutdown exception handling code was calling following method on that npe
          consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage());
          consoleExceptionPrintTrace(exception);
          but the call exception.getMessage() on npe was returning null and consolePropertyMessage is not written to handle null 2nd param for this call of consolePropertyMessage. But the work done by this particular type of consolePropertyMessage method is also done by the next call in the shutdown exception handling which is consoleExceptionPrintTrace. Because of this, I have replaced the consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); call with consolePropertyMessage("DRDA_UnexpectedException.S", true); This change makes sure we do not run into npe when exception.getMessage() is null. I am not sure if we should enter a new jira entry to make consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); handle a null 2nd param.

          i ran junit tests and then ran into intermittent upgrade test failures. i am running derbyall and as soon as they are finished, i will go ahead and checkin the patch.

          One last step remaining is may be write a new method which is a subset of consolePropertyMessage because consolePropertyMessage handles the exception printing and then goes ahead and throws the exception again. For our purposes in this jira entry, we do not want the exception to be thrown back. instead, we want to move on to the next step in server shutdown.

          Show
          Mamta A. Satoor added a comment - I am attaching(DERBY4304_fixNPE_patch2_diff.txt) a simple fix to NPE problem. During one of the shutdown steps, DERBY-4306 was throwing a npe(we have fixed DERBY-4306 now) and the shutdown exception handling code was calling following method on that npe consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); consoleExceptionPrintTrace(exception); but the call exception.getMessage() on npe was returning null and consolePropertyMessage is not written to handle null 2nd param for this call of consolePropertyMessage. But the work done by this particular type of consolePropertyMessage method is also done by the next call in the shutdown exception handling which is consoleExceptionPrintTrace. Because of this, I have replaced the consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); call with consolePropertyMessage("DRDA_UnexpectedException.S", true); This change makes sure we do not run into npe when exception.getMessage() is null. I am not sure if we should enter a new jira entry to make consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); handle a null 2nd param. i ran junit tests and then ran into intermittent upgrade test failures. i am running derbyall and as soon as they are finished, i will go ahead and checkin the patch. One last step remaining is may be write a new method which is a subset of consolePropertyMessage because consolePropertyMessage handles the exception printing and then goes ahead and throws the exception again. For our purposes in this jira entry, we do not want the exception to be thrown back. instead, we want to move on to the next step in server shutdown.
          Mamta A. Satoor made changes -
          Attachment DERBY4304_fixNPE_patch2_diff.txt [ 12414650 ]
          Hide
          Mamta A. Satoor added a comment -

          Committed patch DERBY4304_fixNPE_patch2_diff.txt with following comments (revision 798347)

          DERBY-4304

          During one of the server shutdown steps, DERBY-4306 was throwing a npe(we have fixed DERBY-4306 now) and the shutdown exception handling code was calling following method on that npe
          consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage());
          consoleExceptionPrintTrace(exception);
          but the call exception.getMessage() on npe was returning null and consolePropertyMessage is not written to handle null 2nd param for this call of consolePropertyMessage. But the work done by this particular type of consolePropertyMessage method is also done by the next call in the shutdown exception handling which is consoleExceptionPrintTrace. Because of this, I have replaced the consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); call with consolePropertyMessage("DRDA_UnexpectedException.S", true); This change makes sure we do not run into npe when exception.getMessage() is null. I am not sure if we should enter a new jira entry to make consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); handle a null 2nd param.

          junit tests with this change ran into intermittent upgrade test failures.

          One last step remaining is may be write a new method which is a subset of consolePropertyMessage because consolePropertyMessage handles the exception printing and then goes ahead and throws the exception again. For our purposes in this jira entry, we do not want the exception to be thrown back. instead, we want to move on to the next step in server shutdown.

          Show
          Mamta A. Satoor added a comment - Committed patch DERBY4304_fixNPE_patch2_diff.txt with following comments (revision 798347) DERBY-4304 During one of the server shutdown steps, DERBY-4306 was throwing a npe(we have fixed DERBY-4306 now) and the shutdown exception handling code was calling following method on that npe consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); consoleExceptionPrintTrace(exception); but the call exception.getMessage() on npe was returning null and consolePropertyMessage is not written to handle null 2nd param for this call of consolePropertyMessage. But the work done by this particular type of consolePropertyMessage method is also done by the next call in the shutdown exception handling which is consoleExceptionPrintTrace. Because of this, I have replaced the consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); call with consolePropertyMessage("DRDA_UnexpectedException.S", true); This change makes sure we do not run into npe when exception.getMessage() is null. I am not sure if we should enter a new jira entry to make consolePropertyMessage("DRDA_UnexpectedException.S", exception.getMessage()); handle a null 2nd param. junit tests with this change ran into intermittent upgrade test failures. One last step remaining is may be write a new method which is a subset of consolePropertyMessage because consolePropertyMessage handles the exception printing and then goes ahead and throws the exception again. For our purposes in this jira entry, we do not want the exception to be thrown back. instead, we want to move on to the next step in server shutdown.
          Hide
          Mamta A. Satoor added a comment -

          With revision, committed 798742, made changes so that during server shutdown sub-steps, we print the exception info and then move on to the next step in server shutdown.The commit comments were as follows
          *****************
          During server shutdown substeps, catch any exception thrown, print it to console along with stack trace and then move on to the next substep rather than re-throwing the exception after console printing.
          *****************

          Show
          Mamta A. Satoor added a comment - With revision, committed 798742, made changes so that during server shutdown sub-steps, we print the exception info and then move on to the next step in server shutdown.The commit comments were as follows ***************** During server shutdown substeps, catch any exception thrown, print it to console along with stack trace and then move on to the next substep rather than re-throwing the exception after console printing. *****************
          Hide
          Mamta A. Satoor added a comment -

          Attaching a new patch DERBY4304ShutdownException_patch3_diff.txt which puts a new try catch block around all of the server shutdown code in case if there is some exception being thrown which is not getting caught. The catch block will log into the log file but if that fails for some reason, it will also just dump the stack trace using ex.printStackTrace().

          I have attached for reference the server console log with my changes logAfterPatch3Changes.txt and without my changes logBeforePatch3Changes.txt. i do not see any new exception in the log file when running junit suite. I will also run derbyall. If no one has any feedback to this patch, I will go ahead and commit it tomorrow.

          Show
          Mamta A. Satoor added a comment - Attaching a new patch DERBY4304ShutdownException_patch3_diff.txt which puts a new try catch block around all of the server shutdown code in case if there is some exception being thrown which is not getting caught. The catch block will log into the log file but if that fails for some reason, it will also just dump the stack trace using ex.printStackTrace(). I have attached for reference the server console log with my changes logAfterPatch3Changes.txt and without my changes logBeforePatch3Changes.txt. i do not see any new exception in the log file when running junit suite. I will also run derbyall. If no one has any feedback to this patch, I will go ahead and commit it tomorrow.
          Mamta A. Satoor made changes -
          Attachment logAfterPatch3Changes.txt [ 12416269 ]
          Attachment logBeforePatch3Changes.txt [ 12416270 ]
          Attachment DERBY4304ShutdownException_patch3_diff.txt [ 12416271 ]
          Hide
          Kathey Marsden added a comment -

          Hi Mamta,

          I took a quick look at the diff and noticed that the normal timestamps that print for network server commands are commented out, which I guess you did for your testing. These should be enabled again before commit.

          The rest of the diff I think is ok but was a little hard to read because of the indentation change. Thanks for doing this. I think this will finally allow us to get full reporting of any shutdown issues. It would be great to see this change backported, but you would need to be careful to also backport the fixes for the bugs found when fixing this issue as well.

          Kathey

          Show
          Kathey Marsden added a comment - Hi Mamta, I took a quick look at the diff and noticed that the normal timestamps that print for network server commands are commented out, which I guess you did for your testing. These should be enabled again before commit. The rest of the diff I think is ok but was a little hard to read because of the indentation change. Thanks for doing this. I think this will finally allow us to get full reporting of any shutdown issues. It would be great to see this change backported, but you would need to be careful to also backport the fixes for the bugs found when fixing this issue as well. Kathey
          Hide
          Mamta A. Satoor added a comment -

          Thanks for reviewing the changes, Kathey. Yes, I have undone changes for skipping the timestamp printing. I have committed the rest of the changes with revision 803548. Once the derbyall run is finished, I will look into backporting the changes.

          Show
          Mamta A. Satoor added a comment - Thanks for reviewing the changes, Kathey. Yes, I have undone changes for skipping the timestamp printing. I have committed the rest of the changes with revision 803548. Once the derbyall run is finished, I will look into backporting the changes.
          Hide
          Mamta A. Satoor added a comment -

          Merged the changes for this jira entry into 10.5 codeline using revision r808479. Will work on backporting to 10.4 codeline next.

          Show
          Mamta A. Satoor added a comment - Merged the changes for this jira entry into 10.5 codeline using revision r808479. Will work on backporting to 10.4 codeline next.
          Mamta A. Satoor made changes -
          Fix Version/s 10.5.3.1 [ 12314182 ]
          Fix Version/s 10.6.0.0 [ 12313727 ]
          Hide
          Mamta A. Satoor added a comment -

          Merged 5 out of 6 checkins that went in for DERBY-4304 into 10.4 codeline
          svn merge -r 794302:794303 https://svn.apache.org/repos/asf/db/derby/code/trunk/
          svn merge -r 796315:796316 https://svn.apache.org/repos/asf/db/derby/code/trunk/
          svn merge -r 796371:796372 https://svn.apache.org/repos/asf/db/derby/code/trunk/
          svn merge -r 798346:798347 https://svn.apache.org/repos/asf/db/derby/code/trunk/
          svn merge -r 798741:798742 https://svn.apache.org/repos/asf/db/derby/code/trunk/

          Will mege the last one as a separate checkin.

          Show
          Mamta A. Satoor added a comment - Merged 5 out of 6 checkins that went in for DERBY-4304 into 10.4 codeline svn merge -r 794302:794303 https://svn.apache.org/repos/asf/db/derby/code/trunk/ svn merge -r 796315:796316 https://svn.apache.org/repos/asf/db/derby/code/trunk/ svn merge -r 796371:796372 https://svn.apache.org/repos/asf/db/derby/code/trunk/ svn merge -r 798346:798347 https://svn.apache.org/repos/asf/db/derby/code/trunk/ svn merge -r 798741:798742 https://svn.apache.org/repos/asf/db/derby/code/trunk/ Will mege the last one as a separate checkin.
          Mamta A. Satoor made changes -
          Fix Version/s 10.4.2.1 [ 12313401 ]
          Hide
          Mamta A. Satoor added a comment -

          Merged the final remaining changes revision 803548 from trunk into 10.4 codeline. The merged happened with revision 809593.

          Show
          Mamta A. Satoor added a comment - Merged the final remaining changes revision 803548 from trunk into 10.4 codeline. The merged happened with revision 809593.
          Hide
          Mamta A. Satoor added a comment -

          Backported changes into 10.3 codeline with revision 810956.

          Show
          Mamta A. Satoor added a comment - Backported changes into 10.3 codeline with revision 810956.
          Mamta A. Satoor made changes -
          Fix Version/s 10.3.3.1 [ 12313143 ]
          Mamta A. Satoor made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Kathey Marsden made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Workflow jira [ 12470387 ] Default workflow, editable Closed status [ 12799083 ]

            People

            • Assignee:
              Mamta A. Satoor
              Reporter:
              Mamta A. Satoor
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development