glassfish
  1. glassfish
  2. GLASSFISH-18948

The error message isn't friend to user while deploying a rar the size of which is larger than current allocated java heap size

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0_b45
    • Fix Version/s: 4.0_b51
    • Component/s: deployment
    • Labels:
      None
    • Environment:

      Windows Xp

      Description

      [Bug Description]
      The error message isn't friend to user while deploying a rar the size of which is larger than current allocated java heap size

      [Operations]
      1、Prepare a rar which contains a large file(this file's size is larger than current allocated java heap size).

      2、While deploying the rar using GUI or command, the following error message is displayed on cmd:

      ""Jave heap size""

      [revise way]
      let user know that the current deployed file is larger than current allocated java heap size.

      [Affected versions ]
      1 4.0_b45
      2 gf's trunk until 2012/07/24

      1. screenshot-1.jpg
        141 kB
      2. screenshot-2.jpg
        146 kB

        Activity

        Jeremy_Lv created issue -
        Hide
        Jeremy_Lv added a comment -

        The glassfish throw out the error messages(java heap space) defined in JDK on GUI.
        This error message isn't friendly to user .

        Show
        Jeremy_Lv added a comment - The glassfish throw out the error messages(java heap space) defined in JDK on GUI. This error message isn't friendly to user .
        Jeremy_Lv made changes -
        Field Original Value New Value
        Attachment screenshot-1.jpg [ 50693 ]
        Hide
        Jeremy_Lv added a comment -

        I think the error messages should be more friendly , just as the picture shows

        Show
        Jeremy_Lv added a comment - I think the error messages should be more friendly , just as the picture shows
        Jeremy_Lv made changes -
        Attachment screenshot-2.jpg [ 50694 ]
        Hide
        Jeremy_Lv added a comment -

        I have uploaded my modified code to this improvement.

        Show
        Jeremy_Lv added a comment - I have uploaded my modified code to this improvement.
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50747 ]
        Hide
        Hong Zhang added a comment -

        Thanks for submitting the patch for this one. I think it's good that we report some error message for this, but is the cause of the error always related to heap size, could it be something else?

        And as a general comment for this change and other changes, please make sure the changed files do not contain the windows character "^M" and the identation consistent with the existing code.

        Show
        Hong Zhang added a comment - Thanks for submitting the patch for this one. I think it's good that we report some error message for this, but is the cause of the error always related to heap size, could it be something else? And as a general comment for this change and other changes, please make sure the changed files do not contain the windows character "^M" and the identation consistent with the existing code.
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50747 ]
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50758 ]
        Jeremy_Lv made changes -
        Comment [ } catch(Throwable loadException) {
                logger.log(Level.SEVERE, loadException.getMessage(), loadException);
                report.failure(logger, "Exception while loading the app", null);
                report.setFailureCause(loadException);
                tracker.actOn(logger);
                return null;
            }
        ]
        Hide
        Jeremy_Lv added a comment -

        The error message is only occured in the situation in deploying the application contains a large file which is larger than current allocated java heap size.

        For example:
        Deploy an application which contains a large file(this file's size is larger than current allocated java heap size). If your computer's RAM is 2G, I think the error messages(java heap size) can be thrown out by deploying an application contains a file larger than 700M.

        I notice that copy#FileUtils execute the following code:

        else

        { ByteBuffer byteBuffer = ByteBuffer.allocate(Long.valueOf(size).intValue()); inChannel.read(byteBuffer); byteBuffer.rewind(); outChannel.write(byteBuffer); }

        When excute the statement of "ByteBuffer byteBuffer = ByteBuffer.allocate(Long.valueOf(size).intValue());", you will find out the truth that the size may larger than the ByteBuffer can be allocated. After that, the error messages(Java heap size) are thrown out.

        【My opition】
        I think I should catch this error and throw out the messages like "Error while copying file to temp directory;the current deployed file is larger than current allocated Java heap space".

        Show
        Jeremy_Lv added a comment - The error message is only occured in the situation in deploying the application contains a large file which is larger than current allocated java heap size. For example: Deploy an application which contains a large file(this file's size is larger than current allocated java heap size). If your computer's RAM is 2G, I think the error messages(java heap size) can be thrown out by deploying an application contains a file larger than 700M. I notice that copy#FileUtils execute the following code: else { ByteBuffer byteBuffer = ByteBuffer.allocate(Long.valueOf(size).intValue()); inChannel.read(byteBuffer); byteBuffer.rewind(); outChannel.write(byteBuffer); } When excute the statement of "ByteBuffer byteBuffer = ByteBuffer.allocate(Long.valueOf(size).intValue());", you will find out the truth that the size may larger than the ByteBuffer can be allocated. After that, the error messages(Java heap size) are thrown out. 【My opition】 I think I should catch this error and throw out the messages like "Error while copying file to temp directory;the current deployed file is larger than current allocated Java heap space".
        Hide
        Jeremy_Lv added a comment -

        Thanks for your advices, I have updated my modified files and sure about that the changed files don't contains windows character "^M".

        Show
        Jeremy_Lv added a comment - Thanks for your advices, I have updated my modified files and sure about that the changed files don't contains windows character "^M".
        Hide
        Hong Zhang added a comment -

        Maybe the try/catch block should be around the ByteBuffer.allocate call for this purpose, as there could be other exceptions thrown during reading and writing, and the error message would be misleading when it failed due to other purposes.
        And for error message, please make the log string more specific, something like allocate.more.than.java.heap.space=Trying to allocate a buffer bigger than the current allocated Java heap space.

        Thanks for fixing the ^Ms, please fix the code indentation also.

        Show
        Hong Zhang added a comment - Maybe the try/catch block should be around the ByteBuffer.allocate call for this purpose, as there could be other exceptions thrown during reading and writing, and the error message would be misleading when it failed due to other purposes. And for error message, please make the log string more specific, something like allocate.more.than.java.heap.space=Trying to allocate a buffer bigger than the current allocated Java heap space. Thanks for fixing the ^Ms, please fix the code indentation also.
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50758 ]
        Hide
        Jeremy_Lv added a comment -

        Thanks for your comments.

        I have modified the source and updated my attachment files.

        The error messages are changed like allocate.more.than.java.heap.space=Trying to allocate a buffer bigger than the current allocated Java heap space.

        By the way,the try/catch block has changed around the ByteBuffer.allocate call for this purpose.

        Show
        Jeremy_Lv added a comment - Thanks for your comments. I have modified the source and updated my attachment files. The error messages are changed like allocate.more.than.java.heap.space=Trying to allocate a buffer bigger than the current allocated Java heap space. By the way,the try/catch block has changed around the ByteBuffer.allocate call for this purpose.
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50774 ]
        Hide
        Jeremy_Lv added a comment -

        I'll run the QL tests and dev tests after you check the codes.

        Show
        Jeremy_Lv added a comment - I'll run the QL tests and dev tests after you check the codes.
        Hide
        Hong Zhang added a comment -

        Thanks, the changes look good. But can you fix the indentation, using the standard Java coding convention? For example, the catch line should move 4 spaces to the left to align with the try (and the code inside catch block needs to be fixed also), the last three lines of the method should align with try also?

        Show
        Hong Zhang added a comment - Thanks, the changes look good. But can you fix the indentation, using the standard Java coding convention? For example, the catch line should move 4 spaces to the left to align with the try (and the code inside catch block needs to be fixed also), the last three lines of the method should align with try also?
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50774 ]
        Hide
        Jeremy_Lv added a comment -

        I'm sorry about that I haven't using the standard Java coding convention. Now
        I have revised the codes as your comments.
        Please check it again.
        Best Regards.

        Show
        Jeremy_Lv added a comment - I'm sorry about that I haven't using the standard Java coding convention. Now I have revised the codes as your comments. Please check it again. Best Regards.
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50789 ]
        Hide
        Hong Zhang added a comment -

        No problem and thanks for being so patient with me on the review comments

        I looked at the changes, most of the indentations were fixed except the last line (the closing curly brace for the catch block), that line should be moved 4 spaces to the left.

        Show
        Hong Zhang added a comment - No problem and thanks for being so patient with me on the review comments I looked at the changes, most of the indentations were fixed except the last line (the closing curly brace for the catch block), that line should be moved 4 spaces to the left.
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50789 ]
        Hide
        Jeremy_Lv added a comment -

        Thanks for being so kind about checking my changes.
        I have fixed the indentation as your comments(the closing curly brace for the catch block).

        Show
        Jeremy_Lv added a comment - Thanks for being so kind about checking my changes. I have fixed the indentation as your comments(the closing curly brace for the catch block).
        Jeremy_Lv made changes -
        Attachment GLASSFISH-18948_revised_source.zip [ 50801 ]
        Hide
        Hong Zhang added a comment -

        Changes look good. Please run the necessary tests before check in. And please check in all the files in the same svn commit as we discussed through the email.

        Show
        Hong Zhang added a comment - Changes look good. Please run the necessary tests before check in. And please check in all the files in the same svn commit as we discussed through the email.
        Hide
        Jeremy_Lv added a comment -

        Dear Hong:
        I have some dev tests questions:
        for example:the modified file are related to the directory like "GF_main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io", I can't find any profile files like "common" in the directory of "dev_test_main\devtests".
        shall I run all of the dev tests?

        Show
        Jeremy_Lv added a comment - Dear Hong: I have some dev tests questions: for example:the modified file are related to the directory like "GF_main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io", I can't find any profile files like "common" in the directory of "dev_test_main\devtests". shall I run all of the dev tests?
        Hide
        Hong Zhang added a comment -

        No, you don't need to do. As the fix you did was for a negative case and should not be harmful for regular cases, you could just run QuickLook tests.

        Show
        Hong Zhang added a comment - No, you don't need to do. As the fix you did was for a negative case and should not be harmful for regular cases, you could just run QuickLook tests.
        Hide
        Jeremy_Lv added a comment -

        The QL tests results looks OK.

        QL test information:
        Test environment: Linux RHEL5-x32
        Glassfish trunk: until 2012/08/06
        QuickLook tests version: until 2012/08/06
        scenarios: Glassfish Profile
        Passed cases/Tests number: 107/107

        I have uploaded my QL tests results.

        Show
        Jeremy_Lv added a comment - The QL tests results looks OK. QL test information: Test environment: Linux RHEL5-x32 Glassfish trunk: until 2012/08/06 QuickLook tests version: until 2012/08/06 scenarios: Glassfish Profile Passed cases/Tests number: 107/107 I have uploaded my QL tests results.
        Jeremy_Lv made changes -
        Attachment test-output(QL_GF_profile).zip [ 50829 ]
        Hide
        Jeremy_Lv added a comment -

        Sending main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io\FileUtils.java
        Sending main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io\LocalStrings.properties
        Transmitting file data ..
        Committed revision 55370.

        Show
        Jeremy_Lv added a comment - Sending main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io\FileUtils.java Sending main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io\LocalStrings.properties Transmitting file data .. Committed revision 55370.
        Jeremy_Lv made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 4.0_b51 [ 15640 ]
        Resolution Fixed [ 1 ]

          People

          • Assignee:
            Hong Zhang
            Reporter:
            Jeremy_Lv
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: