Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No warning message appears when a large file can't be deleted from File Manager UI #29

Open
ane-gabriela opened this issue Feb 19, 2020 · 7 comments

Comments

@ane-gabriela
Copy link

ane-gabriela commented Feb 19, 2020

In regards to #7

Tested on XWiki 11.10.3 local instance and Cloud 11.3.6 and no error message appears when trying to delete a file of 50MB or larger.

After clicking the Delete button the UI refreshes after some time, the file remains - it's not removed and no error message appears.

NullError

@polx
Copy link

polx commented Feb 27, 2020

I tested 2.2.11 on multiple versions and could not reproduce it systematically.
In some cases I saw out-of-memory-errors mentioned in the console.
In one other case, I saw the error arrive as an object instead of as a string.

In the cases where I managed to reproduce it (only on xwiki 9 or 11), I could see that, as the screenshot shows, the error is not there in the status (the status' error is null).

In many other cases (macOS 10.14 with java 8 with xwiki 8.4, 9.11.9, 10.11.3, Ubuntu Linux with java 11 with xwiki 11.3, Debian Linux with java 8 and xwiki 11.3) I could upload and delete files as large as 600 Megabytes. I had errors at upload but none at delete.

From now I see the following possible choices:

1.- explore at the host of Gabi and debug there hopefully finding a pattern or a software difference
2.- enrich a strategy where the closing function call performs a check to see if the deletion was successful and if not display a message
3.- keep debugging all the possible cases by using the Docker testing (I started to look there then I realised that OOMs are not the really super candidate to try on docker)

My suggestion would be to just do number 2.
@mflorea : What do you think?

@mflorea
Copy link
Collaborator

mflorea commented Feb 28, 2020

@polx I would start with 1. Talk with @ane-gabriela to get more details. Maybe there are some details we're missing or maybe she can give you access to her XWiki instance to test. Thanks.

@polx
Copy link

polx commented Mar 10, 2020

Hello @mflorea : I have now been able to investigate on @ane-gabriela's testing engine and can see the errors very well there.

Diagnostics: Memory management is, again, the problem: I always obtained OOM when duplicating the document's RCS. Do we really need the duplication before delete. It seems that this the cause.

Nonetheless, the error is only collected among the log and not within the error field. I am not sure why this is such and I believe that this may depend on the VM and the XWiki version. So I reiterate my suggestion number 2: enrich a strategy where the closing function call performs a check to see if the deletion was successful and if not display a message.

@polx polx assigned polx and mflorea and unassigned polx Mar 12, 2020
@mflorea
Copy link
Collaborator

mflorea commented Mar 18, 2020

Do we really need the duplication before delete. It seems that this the cause.

I doubt the clone() call is the issue. I bet on the deleteDocument() call. But anyway, it doesn't mater that much where is the source for OOM. The fact is: you can get an exception while deleting a document for many reasons. We need to be able to properly display an error message when that happens. Limiting the cases when the exception is thrown is another issue. So I think we need to investigate why $jobStatus.error is null. Maybe there is something to fix on the delete job.

Making an additional HTTP request at the end to check if the page still exists is not reliable: there is the edge case when a different user re-creates the page at the same time. In other words: the fact that the page exists some time after the delete finished doesn't imply (logically) that the delete has failed. It can simply mean that the page was re-created afterwards.

@mflorea mflorea assigned polx and unassigned mflorea Mar 18, 2020
@polx
Copy link

polx commented Mar 18, 2020

Hello @mflorea ,
I'm afraid everything is happening outside of the filemanager application. Please see the file
jobstatus-oom.xml.zip : the OutOfMemoryError is not thrown: It is just reported. This was produced on a patched FileManager where the runInternal of DeleteJob was enriched by a catch (Throwable t) { status.setError(t); throw t; } before the finally.

One with the released FileManager (2.2.11) is in
oom-jobstatus-release.xml.zip. It is very similar.

In both cases an error is not thrown, the OOM happens in some array-copy down the road of deleteFile within XWikiAttachmentRCSArchive. This might well be outside of clone. You might be right there.

Thus changing the application can only be done with information provided to the user but not with catching an error.

the fact that the page exists some time after the delete finished doesn't imply (logically) that the delete has failed. It can simply mean that the page was re-created afterwards.

The implication is indeed wrong. But the wording could be carefully chosen so as not to confuse "the delete has failed" with "at the end of the delete operation, the file was still there".

Do you see an alternative?

@mflorea
Copy link
Collaborator

mflorea commented Mar 20, 2020

I'm afraid everything is happening outside of the filemanager application.

The OOM stacktrace starts from the File Manager application:

<trace>com.xpn.xwiki.XWiki.deleteDocument(XWiki.java:4415)</trace>                <trace>org.xwiki.filemanager.internal.DefaultFileSystem.delete(DefaultFileSystem.java:193)</trace>
<trace>org.xwiki.filemanager.internal.job.DeleteJob.deleteFile(DeleteJob.java:118)</trace>

so we know the OOM is thrown when a file is deleted. Sure, the exception may be caught somewhere, but the fact that you have it in the job log means you can determine if the job has failed or not (with high probability) by checking if there are log events of level ERROR. We can at least mention to the user that some errors were logged during the delete operation which could mean that the file was not deleted successfully. So instead of simply:

'error': $jobStatus.error,
'log': {},

we should also check the job logs. Either by passing them to the client (but it may be too expensive) or by setting the value of 'error' taking into account also the job logs.

@mflorea mflorea removed their assignment Mar 20, 2020
@polx polx removed this from the 2.2.11 milestone Jul 28, 2020
@ane-gabriela
Copy link
Author

Reproduced on XWiki 10.11.9 local instance - can't delete file and no warning is shown. I get a "java.lang.OutOfMemoryError: Java heap space" message in the XWiki console. Used a file of 44.6 MB

AfterDelete

NOTE: I've also tested on XWiki 12.10.4 with the same file and the documented is deleted, no errors appear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants