Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 353167

Summary: CDOSavePoint and Reattachment issue
Product: [Modeling] EMF Reporter: Stephane fournier <stephane.fournier>
Component: cdo.coreAssignee: Eike Stepper <stepper>
Status: CLOSED FIXED QA Contact: Eike Stepper <stepper>
Severity: normal    
Priority: P3 CC: caspar_d, stepper, steve.monnier
Version: 4.1   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Patch v1 none

Description Stephane fournier CLA 2011-07-26 17:59:43 EDT 
Build Identifier: CDO 4.0 GA

1) Open a CDO Transaction on a resource.
2) Delete one object (dummyObject) from its container.
3) Create a CDOSavePoint
4) Undo the delete operation 
5) Create a CDOSavePoint
6) Modify the dummyObject, let say change one of its attributes for instance its name.
7) Create a CDOSavePoint.
8) Commit the CDO transaction.

>> The modification made at step 6 is not committed due to the fact the dummyObject was contained in getSharedDetachedObjects.

See the post at the following link to have the whole explanation : 
http://www.eclipse.org/forums/index.php/mv/msg/222265/702727/#msg_702727

Stephane.



Reproducible: Always
Comment 1 Eike Stepper CLA 2011-07-27 01:44:45 EDT 
Some infos about the evolution of the relevant CDO features:

1) Initially there was no support for savepoints or reattachment. All commit/rollback data was recorded in CDOTransactionImpl.

2) Later support for savepoints has been added. Basically by factoring the commit/rollback data into CDOSavepointImpl. Reattachment still wasn't considered.

3) Support for reattachment has been added to CDOSavepointImpl.

The savepoint design was "inherited" from the former transaction design, meaning that a single savepoint only contains the delta compared to the preceeding savepoint. None of the following major operations can be executed without looking at several or all savepoints:

- tx.commit()
- tx.rollback()
- tx.getXyzObjects()
- savepoint.rollback()
- savepoint.getXyzObjects()

In addition the following issues are evident:

- Reattachment over multiple savepoints is broken.
- Netting out modifications is broken.
- Preceeding savepoints can not be garbage collected.
- One can not merge into a dirty transaction.
- Passive updates and conflicts are not properly considered with savepoints.
- Many other mechanisms suffer from the savepoint complexity.

In essence, the mechanism is complex, functionally insufficent and often slow and hungry for heap space. Most of the above issues could be addressed by a major redesign of CDOSavepointImpl. All internal maps would be removed and replaced with a single one:

  Map<CDOID, ObjectRecord> objectRecords;

  class ObjectRecord 
  {
    private final CDOState initialState;
    private final CDORevision initialRevision;
    private CDORevisionDelta revisionDelta;
  }

This would be a one or two weeks effort because so many functions rely on the current design.
Comment 2 Eike Stepper CLA 2011-07-27 02:29:25 EDT 
I have a hotfix in mind for this particular problem, though...
Comment 3 Eike Stepper CLA 2011-07-27 02:35:35 EDT 
Created attachment 200415 [details]
Patch v1

This patch removes the sharedDetachedObjects map and changes several getAllXyz() methods in CDOSavepointImpl (turn backward iteration into forward iteration, i.e. "chronological replay").
Comment 4 Eike Stepper CLA 2011-07-27 02:35:50 EDT 
Test cases are in TransactionTest:
testReattachCommit()
testReattachModifyCommit()
testReattachCommitWithSavepoints()
testReattachModifyCommitWithSavepoints()
Comment 5 Eike Stepper CLA 2011-07-27 02:52:19 EDT 
Committed revision 8821:
- trunk/plugins/org.eclipse.emf.cdo
- trunk/plugins/org.eclipse.emf.cdo.tests
Comment 6 Eike Stepper CLA 2011-07-27 02:53:21 EDT 
The hotfix passes all tests. Resolving for now. Let me know if you want any of the other mentioned issues fixed.
Comment 7 Stephane fournier CLA 2011-07-27 08:13:09 EDT 
Is it merged into CDO 4.0 maintenance branch ?
Comment 8 Eike Stepper CLA 2011-07-27 09:59:39 EDT 
Yes, through bug 353172.
Comment 9 Eike Stepper CLA 2012-09-21 07:17:34 EDT 
Closing.