Community
Participate
Working Groups
CDO should provide "revert to the history revision" operation.
I Still have some doubt about this feature. If data is restored to a version of history, but one of its EReferences has been deleted. How should we deal with?
Rebasing all unresolved enhancement requests to 3.0
Just thinking loudly about revert functionality :)... We are about to implement revert in our application. At current time, our logical scope (project) consist from data resource and few management objects outside this resource. Management resources contains version number, comment and author. Hence for us, revert function should include following: - revert of set of cdo objects from specified timestamp - ordinary editing of management data (setting new version, comment and author). All these function should be done in single transaction in order to expose for user as single action. Let say editing of elements along with revert is our application specifics (we could make it with XA transactions), but revert function could be implemented in CDO part. I suppose, that revert api must take CDO object IDs of elements (composited elements shall be inclusive automatically) and timestampe to which must be reverted. There are open questions: !. Where to make physically revert - on server or on client (with proceeding commit)? 2. How to extend API on client? Add new function to CDO transaction "CDOUserTransaction.revertTo(long, Set<CDOID>)" which result shall be new version on server or revert should be physically did by commit?
> !. Where to make physically revert - on server or on client (with proceeding > commit)? If making on client side (regardless if it is CDO client or our application client), then it is required: - to load all CDO objects on client from the specified timestamp (audit view) - load current version (at least we need to collect CDOIDs) - lock current version CDO objects (we don't want to get conflict on committing) - apply object state from audit view to the objects loaded as current state - reload locked IDs, because they could be already changed - commit transaction Easy to say, but: - how to change deleted object to the "alive" again? Is it possible at all to do this on client side? I think no... (Eike could correct me if i am wrong) - trying to receive lock on client for all CDO objects could decrease server respond to other clients dramatically - loading whole project (current version and history version) on client and committing new version is performance penalty too. Taking from performance side, it doesn't seems attractive to have revert on client.
> !. Where to make physically revert - on server or on client (with proceeding > commit)? For server side implementation we should : - send CDOID scope and timestamp to server - lock all CDO ID's from the passed scope - load from DB current revisions and from the specified revision - apply object values (copy) from the revision to the current - write to DB changed revisions All work is done on server and no huge data transferring via network and no locks holding on client side. But there are few open issues: - commit signal changes: somehow we need to pass revert from timestamp and CDOIDs to revert (revert scope) - how to collect composite elements on server, if we don't have CDO objects there? - is it posible restore deleted object to the alive? - will Eike accept such changes on server? :) Despite issues, revert on server side looks better choice than revert on client side.
> - commit signal changes: somehow we need to pass revert from timestamp and > CDOIDs to revert (revert scope) Passing values is not technical problem - all means exist and we need only to add additional signal. But where to create open API? Perhaps it is better to add new method to CDOTransaction *revert(long timestamp, Set<CDOObject> objects, boolean recursively)*... > - how to collect composite elements on server, if we don't have CDO objects > there? We can obtain CDOClassInfo from the "revision.getClassInfo()" on server. And this CDOClassInfo can provide EStructuralFeature by method "getAllPersistentFeatures" and feature by itself has info, if it is containment and what CDOID has as value. Seems to be no problem. > - is it posible restore deleted object to the alive? Logically comes solution to add new record for removed object, where *created* timestamp value == *revert date* and *revised* is "0". Of course, it shall involve adding new API. However such new record will break assumption, that revision *revised* value is equals to the revision's with higher by one version *create* date - 1 (timestamp - 1). For example, such assumption is used in *LRURevisionCache.reviseHolder()*, in *MEMStore.addRevision* and perhaps there are much more of them... Such places should be reviewed > - will Eike accept such changes on server? :) What do you think?
> Passing values is not technical problem - all means exist and we need only to > add additional signal. But where to create open API? Perhaps it is better to > add new method to CDOTransaction *revert(long timestamp, Set<CDOObject> > objects, boolean recursively)*... It is not good idea to invent new method *revert* which should behave like *commit*. It should prevent from using in XA transaction. Better choice could be to invent method *setRevertFrom(long timestamp, Set<CDOObject> objects, boolean recursively)*, which just locally setup values to transaction and later on *commit* revert options will be send to server and reverted along with commit of general changes in transaction.
That sounds better. Inconcistencies should be introduced locally, resolved manually and then activley committed ;-)
(In reply to comment #8) > That sounds better. Inconcistencies should be introduced locally, resolved > manually and then activley committed ;-) Actually, there won't be any inconsistencies - we are reverting to the version, which had been already consistent. We have here mass data update. Why to pump huge data amount (current version and reverted version) back and forward from server to client? It shall involve performance issues on huge projects, because client shall need to load it all (even two copies), just because he want to revert it! Despite this, client shall need to ensure that data is not changed during load-copy-commit operation, else it will be involved in conflict resolving and re-applying changes. Mine idea is to have revert on server side. What we need, is to transfer set of objects to server, which suppose to be reverted and pass revision number (revert from). On server side, we need to collect recursively and lock for writing given CDOID's, load reverting revisions, copy data onto the current revision and write to store, notify other IView's about changes. No data transferring over network, no performance issues (client doesn't need to load whole project on his machine), no huge client side locks.
Pasting skype conversation: Eike Stepper: my main question: is "revert" meant to set a single object (or some) back to a historical state? or the whole repository? Saulius T. : I would say set of objects. Usually our set is one Resource. Saulius T. : we store many projects in the same repository Eike Stepper: i don't like that very much because in general it leads to inconsistencies Saulius T. : inconsistencies might occur only in related data graphs Eike Stepper: yes, and generally these can exist Saulius T. : yes, but I think it is client's responsibility to take care of the right scope Saulius T. : like with remove Saulius T. : we don't force to load whole repository just to delete incoming references Eike Stepper: it is only your special application and the induced data structure that ensures consistency in certain reverts Saulius T. : yes, our special case. but from user point it is normal case to revert just view objects also. Application might worn user about problems if there are some and reject or allow to solve them. Saulius T. : we plan in future support revert not only on whole project, but also part of project if it is safe. Eike Stepper: if we go through normal transactions for these repository modifications i'm willing to think about it ;) Eike Stepper: then, if you spend time on making commits free of inconsistencies, you'd benefit during reverts, too Eike Stepper: ok? Eike Stepper: i must go in some minutes, can you copy this discussion to the bugzilla? Saulius T. : yes, I agree if commits are consistent, full reverts guarantees also it.
- revert is called by user via transaction.commit. User transmit scope and revision to revert to - revert is done on server. Includes notification about changes to other clients, restoring deleted objects by rewriting entries - at the end of transaction data is consistent. For this we need to ensure that references are updated if after revert object was removed (like normal object removing) Where did we stop here?
I think here are major questions, which still are open and stops this feature from development: - revert is done on server - reverting predefined set of CDOID's. Could involve model inconsistency only, if after revert object was removed, but it was referenced from objects which was not reverted. User must ensure to give right scope. - revert is invoked by user on setting revert arguments (Set<CDOID>, timestamp) to transaction and later calling transaction *commit* Eike, what do you think?
I propose to add the following method to CDOObject: public void cdoRevert(CDORevision target); Alternatively: public CDORevisionDelta cdoRevert(CDORevision target);
Correction! Saulius mentioned the desire to revert already detached objects. That'd require to change the proposal as follows: CDORevisionDelta[] CDOTransaction::revertObjects(CDORevision... revisions);
The second proposal is preferrable even if it will turn out that "resurrection" of deleted objects causes major problems (what I for my part do believe).
Created attachment 169335 [details] revert implementation Revert implementation based on user side. I am the only author and I agree to apply EPL to these lines
It is first implementation. It's limitation is that revert must be executed on the closed set of object - no reference pointing to the reverted object from the outside of reverted objects. Looking to the future, revert should be enriched with some callback mechanism, which will give for user the choice what to do, if during revert is spotted model inconsistency, like dangling references.
I have first implemented on CDO 2.0 and just now moved to 3.0, and I had stuck with new Branch and Revision concepts. It could be that I left few bugs during migration :). Also I did some code changes, which perhaps will break a little strategy of revision handling. Now DetachedCDORevision can be not the last versioned revision. Therefore I made its method *setRevised* empty, just in order to quickly adopt to environment. But definitely patch is need to be checked by suspicious Eike's eye :) The thing, which confuses me was Revision and RevisionInfo. Actually it is hard to understand now, who is responsible (Revision or RevisionInfo) for serializing and de-serializing revision via network. DetachedCDORevision has write/read methods which will throw OperationUnsupportedException - looks like it shouldn't be exchanged via server and client. But there are RevisionInfo, which makes read and write logic in his way - if it is "normal" revision, then it calls method from Revision object to serialize, else makes serialization by it's own RevisionInfo rule. In this way I got DetachedCDORevision transported from server to client, when I call "RevisionManager.getRevision" method, but I fail with exception when I call "RevisionManager.getRevisionByVersion" To pass serialization restrictions in smooth way I made few methods static in RevisionInfo and changed LoadRevisionByVersionIndication and LoadRevisionByVersionRequest to be able to transport every revision kind. I think, write layer should be made in one, the same, class level, for clearance. Let say, why DetachedCDORevision shouldn't write his fields by his own. RevisionInfo could manage exchange by calling revision write or read methods, as it does for "normal" revision.
Aha, one more thing. The patch added one more concept - resurrected object. Restored object has state "NEW", but OID is old. This concept is not exposed in Open API, just used in internals.
Created attachment 169484 [details] Patch v2 - formally adjusted
Rebasing all outstanding enhancements requests to version 4.0
Created attachment 173855 [details] Patch v3 Revert adjusted to 4.0 version. There is some mess with version *since* tag. For one methods it was requested to add 3.1 for others 4.0
Ready to review
Created attachment 174116 [details] Patch v4 - not fully reviewed There are some questions I'd like to discuss with Egidijus...
Comment on attachment 174116 [details] Patch v4 - not fully reviewed Patch v4 is corrupted, see bug 319661. I'll submit a new patch as soon as the CVS servers work again. See bug 319659.
Created attachment 174121 [details] Patch v5 - discussion outstanding
Created attachment 175303 [details] Patch v6 Removed code from patch which was already committed, fixed found comments
Created attachment 179105 [details] Patch v7 With LEGACY wrappers, but still legacy test failing
Created attachment 184690 [details] Patch v8 Legacy model passes revert tests
Created attachment 186163 [details] Patch v9 Adoped to latest refactoring
Created attachment 187642 [details] Patch v10 Changed patch accoring the changes to CDO. Part of the patch (Detached revision enchancement) was backported from cdo 3.0 with bug fix 333675
The concern to suspend revert feature from adding to the community, was potential risk of model corruption. Before making changes to revert, I would like to discuss CDO client safeness and ways to ensure it. Eike and Caspar have proposed strategy to make checking on revert functionality. Yes, revert feature by it's nature can spoil model, but my concern is, that not only revert is capable to corrupt client side model and further server side. At current time CDO have some level of model integrity check on partial commits only, but there are other functionality like rollback, conflict merging, legacy support - could they guaranty model integrity? Also could be third part client code which makes some action on model and edit it in non EMF style (custom merging or custom reverting on feature level). If we decide that there are other possibility to corrupt model (other than revert and partial commit), then IMO there is point to make integrity check as default behavior on any commit. If CDO goal is to ensure model integrity, which comes from client, then, it would be the best solution to keep check centric and in one place. It could be option (session level) to disable centric integrity check, but on own client risk What do you think?
Created attachment 187743 [details] Patch v11 - merged from head and slightly reformatted
(In reply to comment #32) The most important truth in your arguments is that integrity should best be ensured in the server. In fact it *can* only be ensured there. Now that CDO 4.0 has this server-side mechanism (which it didn't when this revert effort started), I agree that we should focus on that. It already prevents from stale references and containment corruption. Bidi xref checks may still be missing. Please note that this server option may consume considerable resources during commit processing. I think for a client it may still be important to get early feedback about the things that will go wrong for sure if that can be recognized already on the client. You're right that revert is not the only feature that could cause some corruption. In summary I think we should further enhance the server side commit checks and generalize the client side pre-commit checks. I'll send a proposal for a possible work distribution in a private mail in a minute...
Moving all open enhancement requests to 4.1
Moving all open issues to 4.2. Open bugs can be ported to 4.1 maintenance after they've been fixed in master.
Moving all outstanding enhancements to 4.3
Moving all open enhancement requests to 4.4
Moving all open bugzillas to 4.5.
Moving all unaddressed bugzillas to 4.6.
Moving all open bugs to 4.7
Moving all unresolved issues to version 4.8-
Moving all unresolved issues to version 4.9
Moving to 4.13.
Moving to 4.16. I'm going to implement this now. "Revert" will mean the following: Get the local transaction into a (dirty) state that contains all the changes (i.e., object attachments, modifications and detachments) necessary to reach (after committing the changes) a state of the entire model graph that is identical to the graph's state at a given (earlier) branch point. The new API is: CDOChangeSetData CDOTransaction::revertTo(CDOBranchpoint)
The changes are committed: https://git.eclipse.org/c/cdo/cdo.git/commit/?id=4d6e437df30ffcb6e2df64911945cf28102f9eeb I leave this bug open because I want to expose the new functionality in the CDO Explorer...
Writable online checkouts now have a "Revert to" submenu: https://git.eclipse.org/c/cdo/cdo.git/commit/?id=cce7362734d1d11626db4c1d17f9d291bd468888
