Ticket #1029 (closed defect: fixed)

Opened 14 years ago

Last modified 14 years ago

inconsistency in the tree cache after changing a object's id

Reported by: jmorliaguet Owned by: lregebro
Priority: P1 Milestone: CPS 3.3.7
Component: CPSCore Version: TRUNK
Severity: major Keywords:
Cc:

Description

changing an object's id from 'stefan-ahlman1717' to 'stefan-ahlman' on a document has caused an inconsistency in the portal tree cache.

I removed the tree and rebuilt it, apparently the rename event was not taken into account or the transaction did not succeed (the portal_trees subscriber is registered and enabled in portal_eventservice)

Traceback (innermost last):

    * Module ZPublisher.Publish, line 113, in publish
    * Module ZPublisher.mapply, line 88, in mapply
    * Module ZPublisher.Publish, line 40, in call_object
    * Module Shared.DC.Scripts.Bindings, line 311, in __call__
    * Module Shared.DC.Scripts.Bindings, line 348, in _bindAndExec
    * Module App.special_dtml, line 175, in _exec
    * Module DocumentTemplate.DT_Let, line 75, in render
    * Module DocumentTemplate.DT_Util, line 196, in eval
      __traceback_info__: getList
    * Module <string>, line 0, in ?
    * Module Products.CPSCore.TreesTool, line 520, in getList

KeyError: 'SV/avdelningar/vmt/personal/stefan-ahlman' 

Change History

comment:1 Changed 14 years ago by jmorliaguet

I can't reproduce the error but this has happened when renaming published documents to object ids already taken or reserved.

This has occured on 2 separate instances (zope-2.8.2 / CPS trunk) already when several people were working on the same site.

maybe a funkload stress test with several users who rename document could reveal the problem

comment:2 Changed 14 years ago by lregebro

  • Owner changed from fguillaume to lregebro
  • Status changed from new to assigned

This problem is a side effect of two things:

  1. The tree updates are now done at the end of the transaction. However, they are still done on objects, with the result that a rename now means that you first rename the object, then tell the trees that the object has been deleted and added. Of course, when the three is told the object has been deleted, it takes the objects rpath, and deletes that from the tree. But, this being at the end of the transaction, the objects rpath has CHANGED, and so, nothing is deleted.
  1. The _queue used to hold the events on the tree tool is not a queue at all. A queue is processed from start to finish, however, in this "queue" the events are held in a dictionary, and therefore the order in which they are processed are more or less random. It's not a queue, it's a pile.

comment:3 Changed 14 years ago by lregebro

  • Status changed from assigned to new
  • Owner changed from lregebro to fguillaume

I thought I could fix issue 1 separately, but I changed my mind. The tree-tool needs a refactoring where rpaths instead of objects are used as parameters to calls. This and the queue behaviour needs to be discussed more widely before anything is done.

comment:4 Changed 14 years ago by fguillaume

A reproducible test case would go a long way toward fixing this bug...

comment:5 Changed 14 years ago by lregebro

  • Owner changed from fguillaume to lregebro
  • Status changed from new to assigned

Reproducing it is trivial:

  1. Create some time of folderish object in a workspace. I used a FAQ, becaus it already existed.
  2. Go to the workspace tree and rebuild it to make sure it's up to date. Note that the new object is listed in the tree.
  3. Rename it with "Change object id".
  4. Go back to the tree, notice that it's incorrect. Exactly *how* it is incorrect may change, because of the general randomness of the so called queue. Often both the old name and the new name appears. Sometimes nothing happened and only the old name appears.

As noted, one of the main reasons it doesn't work is because the when TreeCache?.deleteNode(ob) is called, ob is already renamed, and hence the wrong node will be deleted. deleteNode must use the rpath as a parameter, since that's what is used as a key.

comment:6 Changed 14 years ago by fguillaume

Ok I'm reproducing, I'll take a look.

comment:7 Changed 14 years ago by fguillaume

This is due to how delaying of tree cache updates to the end of the transaction has been done, in a way that's actually incorrect.

The current code of the tree cache is not designed to be called for objects that aren't there anymore, it's inherently synchronous. It will need to be refactored quite a bit.

comment:8 Changed 14 years ago by janguenot

The tree cache code has to be changed if it can't be delayed then because the TreeCacheManager? is doing something we can expected it todo, right ?

If it bothers you to do fix this then I can take care of it.

comment:9 Changed 14 years ago by fguillaume

I know how to fix it, I'll be using an additional module dedicated to tree optimizations.

comment:10 Changed 14 years ago by fguillaume

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed by [29001] and [29002].

Note: See TracTickets for help on using tickets.