Ticket #7223 (closed Bug: wontfix)

Opened 7 years ago

Last modified 3 years ago

Versioning of large files fails (maximum recursion depth exceeded)

Reported by: buehlmann Owned by:
Priority: major Milestone: 3.3.x
Component: Versioning Version:
Keywords: blobs, black-belt, TuneUp42 Cc: dreamcatcher, runyaga, deo, grahamperrin, naro, hoss

Description

The versioning of large files fail:

  • Module ZPublisher.Publish, line 119, in publish
  • Module ZPublisher.mapply, line 88, in mapply
  • Module ZPublisher.Publish, line 42, in call_object
  • Module Products.CMFPlone.FactoryTool, line 376, in call
  • Module ZPublisher.mapply, line 88, in mapply
  • Module ZPublisher.Publish, line 42, in call_object
  • Module Products.CMFFormController.FSControllerPageTemplate, line 90, in call
  • Module Products.CMFFormController.BaseControllerPageTemplate, line 28, in _call
  • Module Products.CMFFormController.ControllerBase, line 231, in getNext
  • Module Products.CMFFormController.Actions.TraverseTo, line 38, in call
  • Module ZPublisher.mapply, line 88, in mapply
  • Module ZPublisher.Publish, line 42, in call_object
  • Module Products.CMFFormController.FSControllerPythonScript, line 106, in call
  • Module Products.CMFFormController.ControllerBase, line 231, in getNext
  • Module Products.CMFFormController.Actions.TraverseTo, line 38, in call
  • Module ZPublisher.mapply, line 88, in mapply
  • Module ZPublisher.Publish, line 42, in call_object
  • Module Products.CMFFormController.FSControllerPythonScript, line 106, in call
  • Module Products.CMFFormController.ControllerBase, line 231, in getNext
  • Module Products.CMFFormController.Actions.TraverseTo, line 38, in call
  • Module ZPublisher.mapply, line 88, in mapply
  • Module ZPublisher.Publish, line 42, in call_object
  • Module Products.CMFFormController.FSControllerPythonScript, line 106, in call
  • Module Products.CMFFormController.ControllerBase, line 231, in getNext
  • Module Products.CMFFormController.Actions.TraverseTo, line 38, in call
  • Module ZPublisher.mapply, line 88, in mapply
  • Module ZPublisher.Publish, line 42, in call_object
  • Module Products.CMFFormController.FSControllerPythonScript, line 104, in call
  • Module Products.CMFFormController.Script, line 145, in call
  • Module Products.CMFCore.FSPythonScript, line 140, in call
  • Module Shared.DC.Scripts.Bindings, line 313, in call
  • Module Shared.DC.Scripts.Bindings, line 350, in _bindAndExec
  • Module Products.CMFCore.FSPythonScript, line 196, in _exec
  • Module None, line 18, in update_version_on_edit <FSControllerPythonScript at /0large_file/update_version_on_edit used for /0large_file/skriptorium.4teamwork.ch.fs> Line 18
  • Module Products.CMFEditions.CopyModifyMergeRepositoryTool, line 282, in save
  • Module Products.CMFEditions.CopyModifyMergeRepositoryTool, line 411, in _recursiveSave
  • Module Products.CMFEditions.ArchivistTool, line 240, in prepare
  • Module Products.CMFEditions.ArchivistTool, line 195, in _cloneByPickle

RuntimeError: maximum recursion depth exceeded

This can be reproduced by activating the versioning with files and when uploading a file of ca. 40MB.

Change History

comment:1 Changed 6 years ago by alecm

  • Cc dreamcatcher, runyaga, deo added

Any chance we can get the Enfold ZVC fixes committed, so we don't have to solve this twice?

comment:2 Changed 6 years ago by deo

Hey Alec,

the idea is to move all the changes we did in the Enfold's ZVC back to trunk on svn.zope.org.

The problem is that right now it may fixes the _cloneByPickle issue but will break half dozen of other tests from CMFEditions... :-(

Sidnei was able to fix a couple of them, but there's still some work to do.

Let's see what happens in the coming weeks...

comment:3 Changed 6 years ago by alecm

This appears to be related to the following Zope bug:  https://bugs.launchpad.net/zope2/+bug/143899 And seems to be related to a fundamental issue in the recursive implementation of cPickle. The solution of using zexp export/import makes sense, but seems to impose requirements on the objects to be copied (having a _p_oid and _p_jar) which aren't met by the wrappers created on the fly by CMFEditions.

comment:4 Changed 6 years ago by alecm

  • Milestone changed from 3.0.x to 3.1

OK, I've committed a workaround to CMFEditions and a migration to enable it to Plone. It works for the File and Image types only by default, but enabling it for custom types is relatively easy (just add a new AbortVersioningOfLargeFilesAndImages modifier in portal_modifiers and change the condition (and possibly the field_names), or change the existing modifier. This will abort all versioning when large files are encountered and display a nice message to that effect.

There's an alternate modifier (disabled by default) which just replaces the file with a placeholder and replaces it with a reference to the file from the working copy on retrieval. This will allow the object to be versioned, but only the non-large-file data. There is currently no user feedback given indicating that the file was not versioned, primarily because crafting a clear and concise message to describe the result and effects is no simple task. This may still be a better behavior for some usecases, so it is available and similarly configurable.

I'm moving this issue to 3.1, which is the earliest we might be able to fix this properly and allow for versioning of large in ZODB objects.

comment:5 Changed 6 years ago by alecm

Regarding the new ZVC. Once there's a ZVC branch available with the new deep copy mechanis (and preferably some clear release goals), I will branch CMFEditions to make things compatible. Gregoire always intended to replace the ZVC backend with a simpler one IIRC, so if that turns out to be easier that may be the way to go.

I do worry about the internal cloneByPickle implementation in CMFEditions, as it relies deeply on using the persistent_id and persistent_load features of the Pickle module. I would probably need to replicate that functionality in the serialization mechanism on the CMFEditions side, which means it would entail far more than just reusing or copying the new cloneByPickle from ZVC. But I promise to give it a try once this new ZVC is vaguely official.

comment:6 Changed 6 years ago by hannosch

  • Milestone changed from 3.1 to 3.2

comment:7 Changed 6 years ago by hannosch

  • Milestone changed from 3.2 to 3.x

comment:8 Changed 6 years ago by hannosch

  • Owner alecm deleted

comment:9 Changed 5 years ago by grahamperrin

  • Cc grahamperrin added

comment:10 Changed 5 years ago by cewing

Just to update, I've just stumbled onto this problem as well. My site is plone 3.1.7, the version of CMFEditions i've got is 1.1.7. For us, the problem cropped up when uploading a ppt file of 6.5MB. Unfortunately, I'm running an OS X server, and the python there throws no errors and simply falls over, so I lost my site twice today before I managed to figure out what was going on, and stop the user from attempting to upload the file again.

Tracing the issue, it appeared to me to crop up in the script 'update_version_on_edit', since that was the script being called every time the problem manifests. Disabling versioning for file-type objects has temporarily solved this problem, but it would be nice to get a real fix in. Anyone still working on this?

C

comment:11 Changed 4 years ago by jluvsu2

  • Keywords TuneUp23 added

comment:12 Changed 4 years ago by jluvsu2

  • Keywords TuneUp24 added; TuneUp23 removed

comment:13 Changed 4 years ago by naro

  • Cc naro added

comment:14 Changed 4 years ago by alecm

I'm surprised to hear that someone has seen a failure on a 6.5 MB file, since that should not be large enough to trigger the recursion depth issues. There shouldn't be enough linked PData objects on a file that size unless the recursion limit on your system has been set artificially low. It would be helpful if you could reproduce the issue with a file this size on a non-OS X systems so a real traceback could be obtained. That way we would know if the crash you're seeing is due to the same cause or some other more tractable issue.

This probably isn't the best TuneUp task, since it involves either completely rewriting the CMFEditions object cloning mechanisms, or fixing python's pickler implementation to handle recursion more gracefully.

There is some light at the end of this tunnel though. The latest releases of CMFEditions support BLOBs and large files stored using a BLOB storage can be versioned without any of these problems. In other words, this is essentially fixed for the File and Image types in Plone 4.0, and can be fixed in earlier versions by using plone.app.blob along with the latest CMFEditions release.

comment:15 Changed 4 years ago by naro

I'm experiencing this problem on versioning custom folderish content type, which contains subfolders, subitems etc. Is this the same problem ? Can (should) folderish types be versioned ? I need to track changes of folder properties, not objects stored inside the folder.

comment:16 Changed 4 years ago by alecm

It's a similar problem, but much more easily fixed (and already fixed automatically in Plone 4.0). You need to enable one of the special folder modifiers for your type in portal_modifiers. I would suggest going to portal_modifier in the ZMI, then choose the OMOutsideChildrensModifier and change the tal expression that enables it to include your type, e.g. "python: portal_type in ('Folder', 'MyCustomFolder').

This will ensure that the versioning system understands that your folder is a folder and that its children should not be considered versionable attributes. If your folder is a BTree based folder, you should try upgrading your CMFEditions to the latest 2.0b1 version, which has fixes for custom BTree folders and also does not require these modifier changes.

comment:17 Changed 4 years ago by jluvsu2

  • Keywords TuneUp25 added; TuneUp24 removed

comment:18 Changed 4 years ago by witsch

  • Keywords blobs added

comment:19 Changed 4 years ago by jluvsu2

  • Keywords TuneUp26 added; TuneUp25 removed

comment:20 Changed 4 years ago by jluvsu2

  • Keywords TuneUp27 added; TuneUp26 removed

comment:21 Changed 4 years ago by jessilfp

  • Keywords TuneUp28 added; TuneUp27 removed

comment:22 Changed 4 years ago by jessilfp

  • Keywords TuneUp29 added; TuneUp28 removed

comment:23 Changed 4 years ago by jessilfp

  • Keywords TuneUp30 added; TuneUp29 removed

comment:24 Changed 4 years ago by hoss

I've just hit this head-on, with a RichDocument instance that has multiple attachment objects of substantial size (10-40MB).

It has essentially ground content work to a halt, so I need a fix asap.

Is the OMOutsideChildrensModifier work-around appropriate for this scenario, since it is perhaps RichDocument's folderish but not really folderish nature that is triggering the bug?

I'm on plone 3.3.2 with CMFEditions 1.2.4. Doing any wholesale plone upgrades (like to v4.*) or converting to blobs is not an option at this time.

Thanks!

comment:25 Changed 4 years ago by hoss

As a follow-up: we experienced a hard zope crash that seemed to correspond to this bug. Is that consistent with what you guys know about this? Interestingly, we can trigger the bug without crashing zope, so it's not necessarily correlated, but I can't find any other reason for the zope crash, and the timing in the logs make a convincing argument.

comment:26 Changed 4 years ago by hoss

  • Cc hoss added

comment:27 Changed 4 years ago by alecm

I suggest you go to the portal_modifiers in the ZMI and add a new modifier to abort versioning of overly large attachment objects. If you have a recent CMFEditions, then there should already be such a modifier applied to File and Image, you should be able to simply add FileAttachment and ImageAttachment to the condition for that modifier.

comment:28 Changed 4 years ago by hoss

Thanks alecm. I was totally unfamiliar with the portal_modifier stuff. Upon inspection, I think I'm going to go with the OMOutsideChildrensModifier for RichDocument, rather than, say, SkipVersioningOfLargeFilesAndImages for FileAttachment and ImageAttachment.

I think I don't really want any versioning of the attachment types when changes are made to the parent RichDocument, so the strict exclusion given by OMOutsideChildrensModifier seems like the better fit.

And, unless I'm mistaken, then when/if changes are actually made to FileAttachment and ImageAttachment objects, they'll version normally.

Actually, I think I just talked myself into using both. Include RichDocument in OMOutsideChildrensModifier to ensure it's treated as truly folderish, and then include FileAttachment/ImageAttachment in SkipVersioningOfLargeFilesAndImages so that the size threshold is applied to them the same as File/Image.

Eventually I'll be on plone 4 and blobs, and this whole mess will go away (fingers crossed). Though I assume portal_modifier still has a vital role in plone 4. Even if it worked as intended I would probably like to avoid needlessly versioning child objects and bloating the storage (blob or otherwise).

comment:29 Changed 4 years ago by jessilfp

  • Keywords TuneUp added; TuneUp30 removed

comment:30 Changed 4 years ago by eleddy

  • Keywords blobs, black-belt added; blobs removed

comment:31 Changed 3 years ago by jessilfp

  • Keywords TuneUp removed

comment:32 Changed 3 years ago by davisagli

  • Status changed from new to closed
  • Keywords black-belt, TuneUp42 added; black-belt removed
  • Resolution set to wontfix

If I'm reading correctly this is resolved in Plone 4 via the use of blobs.

comment:33 Changed 3 years ago by hoss

davisagli - I would only call that a 'workaround', rather than 'resolution', unless use of blobs in P4 is mandatory. If it's optional, then anyone not using blobs is still exposed to the underlying bug.

Note: See TracTickets for help on using tickets.