[
https://issues.apache.org/jira/browse/IGNITE-4188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16184162#comment-16184162
]
Ryabov Dmitrii commented on IGNITE-4188:
----------------------------------------
[~vozerov], 2.4.
[~sboikov], thank you for review.
* rmvLocks is needed to handle some races with concurrent locks/unlocks, since
rollback to savepoint is synchronous operation there is no need to update
rmvLocks in this case. So there is no need to add savepointOrder in
GridCacheVersion, please revert this, just use 'forSavepoint' request flag to
skip rmvLocks update
I removed savepointOrder, and tests don't fail. I found that hang doesn't
happen because of clearing txMap, which I made after savepointOrpder. So there
is no need to skip rmvLocks updates. At least not now.
* to add flags in messages we usually use byte, not boolean (look at
GridDistributedTxPrepareRequest.flags)
Fixed.
* all new methods related to savepoint should be moved to GridNearTxLocal, not
in IgniteTxLocalAdapter, also move awaitLastFuture there from
TransactionProxyImp, TransactionProxyImp should be simple delegating facade
Fixed.
* it seems you copy/pasted SavepointUnlockFuture without understanding what
this code is supposed to do. Why you need all these MiniFutures? why onResult
method is never called? why deadlock detection is needed there?
Yes, i copy/pasted it and left some things for the later, if we will need them.
I'll refactor this class and remove not used things.
* you implemented timeout support in SavepointUnlockFuture, please add
corresponding test (to simulate timeout you can delay message using
TestRecordingCommunicationSpi)
Ok, I'll do it.
* if I add 3 savepoints: 1, 2, 3, 4 and then rollback to 2, shouldn't we remove
savepoints 2, 3, 4? are there such tests?
TxSavepointsSelfTest.testFailRollbackToSavepoint() tests this case.
testOverwriteSavepoints() tests similar case when we overwrite old savepoint
(and remove all afterward savepoints). testMultipleRollbackToSavepoint() tests
when we want rollback to the same savepoint.
I made savepoints as in samples in ticket description, where savepoint stay
alive after rollback and available to rollback again. My bad - I din't noticed
this in description. But do we really need to destroy savepoint? If user
created stable state by savepoint, why he should create another savepoint every
time he rollback here?
* is there test which checks 'unlock' after key's primary or backup node failed?
* need add tests with near cache enabled
I'll do it.
* in tests you also need check that state was correctly restored using
cache.get()
* Done.
> Savepoints support inside of Ignite Transactions
> ------------------------------------------------
>
> Key: IGNITE-4188
> URL: https://issues.apache.org/jira/browse/IGNITE-4188
> Project: Ignite
> Issue Type: Task
> Reporter: Denis Magda
> Assignee: Ryabov Dmitrii
> Fix For: 2.4
>
>
> A savepoint is a special mark inside a transaction that allows all commands
> that are executed after it was established to be rolled back, restoring the
> transaction state to what it was at the time of the savepoint.
> Here is a reference to the similar functionality implemented by some of RDBMs
> vendors.
> https://www.postgresql.org/docs/8.1/static/sql-savepoint.html
> https://docs.oracle.com/cd/B19306_01/server.102/b14200/statements_10001.htm
> http://dev.mysql.com/doc/refman/5.7/en/savepoint.html
> Consider the following example.
> {code}
> BEGIN;
> INSERT INTO table1 VALUES (1);
> SAVEPOINT my_savepoint;
> INSERT INTO table1 VALUES (2);
> ROLLBACK TO SAVEPOINT my_savepoint;
> INSERT INTO table1 VALUES (3);
> COMMIT;
> {code}
> The execution result must guarantee that only values 1 and 3 are inserted
> into table1.
> In Ignite, it should be supported this way (preserving the same behavior as
> above).
> {code}
> Ignite ignite = ....;
> IgniteCache<Integer, Integer> c = ....;
> try (Transaction tx = ignite.transactions().txStart()) {
> c.put(1, 1);
>
> tx.savepoint("mysavepoint");
>
> c.put(2, 2);
>
> tx.rollbackToSavepoint("mysavepoint");
>
> c.put(3, 3);
>
> tx.commit();
> }
> {code}
> As a summary the following has to be supported on Ignite side:
> - The {{savepoint}} method which will set a named transaction savepoint with
> a name of an identifier.
> - Multiple savepoints defined within a transaction. The names of the
> savepoints have to differ from each other. If the current transaction has a
> savepoint with the same name, the old savepoint is deleted and a new one is
> set.
> - The {{rollbackToSavepoint}} method that will roll back all the changes done
> after a specific checkpoint establishment.
> - The {{releaseCheckpoint}} method that will destroy a savepoint, keeping the
> effects of commands executed after it was established.
> - Full support of the behavior listed above at the level of ODBC and JDBC
> drivers and DML (will be handled under separate tickets).
> - The behavior has to be support for all transactional modes.
> Original proposal on the dev list:
> http://apache-ignite-developers.2346864.n4.nabble.com/TX-savepoints-td12041.html
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
