Issue Details (XML | Word | Printable)

Key: GLASSFISH-18948
Type: Improvement Improvement
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Hong Zhang
Reporter: Jeremy_Lv
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
glassfish

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

Created: 26/Jul/12 08:18 AM   Updated: 10/Aug/12 03:05 AM   Resolved: 10/Aug/12 03:05 AM
Component/s: deployment
Affects Version/s: 4.0_b45
Fix Version/s: 4.0_b51

Time Tracking:
Not Specified

File Attachments: 1. Zip Archive GLASSFISH-18948_revised_source.zip (14 kB) 07/Aug/12 03:08 AM - Jeremy_Lv
2. Zip Archive test-output(QL_GF_profile).zip (86 kB) 09/Aug/12 02:24 AM - Jeremy_Lv

Image Attachments:

1. screenshot-1.jpg
(141 kB)

2. screenshot-2.jpg
(146 kB)
Environment:

Windows Xp


Tags:
Participants: Hong Zhang and Jeremy_Lv


 Description  « Hide

[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



Jeremy_Lv added a comment - 09/Aug/12 02:26 AM

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 added a comment - 09/Aug/12 02:24 AM

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.


Hong Zhang added a comment - 08/Aug/12 03:51 PM

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.


Jeremy_Lv added a comment - 08/Aug/12 10:28 AM

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?


Hong Zhang added a comment - 07/Aug/12 02:28 PM

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.


Jeremy_Lv added a comment - 07/Aug/12 03:08 AM

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).


Hong Zhang added a comment - 06/Aug/12 03:47 PM

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 added a comment - 06/Aug/12 01:41 AM

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.


Hong Zhang added a comment - 03/Aug/12 02:30 PM

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 added a comment - 03/Aug/12 03:45 AM

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


Jeremy_Lv added a comment - 03/Aug/12 03:44 AM

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.


Hong Zhang added a comment - 02/Aug/12 03:28 PM

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 added a comment - 02/Aug/12 09:41 AM

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


Jeremy_Lv added a comment - 02/Aug/12 05:57 AM

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".


Hong Zhang added a comment - 01/Aug/12 02:21 PM

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 added a comment - 01/Aug/12 10:25 AM

I have uploaded my modified code to this improvement.


Jeremy_Lv added a comment - 26/Jul/12 08:32 AM

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


Jeremy_Lv added a comment - 26/Jul/12 08:28 AM

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