Orient 3.0 - No automatic rollbacks when exception happens

[careerscale]

OrientDB Version: 3.0

Java Version: 1.8

OS: Win 7

Expected behavior

We are using gremlin api (TP 3.0) and everything working fine with 3.0 with latest snapshots.

one issue we see is that we need to add rollback as a manual step, but as per my understanding,
graph.rollback() is not required (also we removed the graph.begin() as it seemed implicit)

Actual behavior

partial data is getting saved without proper rollback.

Steps to reproduce

Unit test to follow

[careerscale]

@tglman , @lvca could you please suggest what should be the strategy for my team? should we add graph.rollback() on exception? we can wait as needed if this gets resolved.

[wolf4ood]

Hi @careerscale

do you have a test case for this?

[careerscale]

@maggiolo00
https://github.com/careerscale/orientdb-demo/blob/master/orientdb-java-sample/src/test/java/com/orientdb/samples/test/TransactionTest.java

looks like only OValidationException is resulting in proper rollbacks. other scenarios are not working.

there are 2 test cases, one for positiive (working) and other for negative.

[wolf4ood]

Hi @careerscale

the graph.close() currently commit the current tx if any.

In your second test case testDbTransactionOperations_Not_Working

the first addVertex is ok so when the graph is close that vertex is committed.

The missing part is this way of configuring the TX using TP3 api.

See here

http://tinkerpop.apache.org/docs/current/reference/#transactions

[careerscale]

@maggiolo00 , I am not sure if I got it. I modified the code a bit to use factory.getTx() but that didnt make any difference.

Essentially when IllegalStateException happens, I do not see rollback happening.

WARNING: MaxDirectMemorySize JVM option is not set or has invalid value, that may cause out of memory errors. Please set the -XX:MaxDirectMemorySize=16080m option when you start the JVM. java.lang.IllegalStateException at com.orientechnologies.orient.core.sql.parser.OLocalResultSet.next(OLocalResultSet.java:48) at com.orientechnologies.orient.core.sql.parser.OLocalResultSetLifecycleDecorator.next(OLocalResultSetLifecycleDecorator.java:47) at com.orientdb.samples.test.TransactionTest.testDbTransactionOperations_Not_Working(TransactionTest.java:53) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)

[wolf4ood]

hi @careerscale rollback aren't automatic.
When you close a graph instance if there is something in the TX, the close call the commit.

[careerscale]

@maggiolo00, I think as per my previous conversations, I was told we dont need to add rollback explicitly. Actually we made the change in our project code but that results me at times, "No Transaction Active" exception randomly.

[careerscale]

and when we hit this "No Transaction Active" exception, we will not be able to get out of it, we needed to restart DB.
Empirical observation on what works. :-)

[wolf4ood]

Hi @careerscale

with manual rollback you got No Transaction Active. If there is an error in the tx if no manual rollback is called and in finally you close the tx, it gets committed on close.

Once the TP3 lifecycle is implemented correctly you can configure that on close the tx is rollbacked

[careerscale]

@maggiolo00
Once the TP3 lifecycle is implemented correctly .. did you mean in OrientDB or in the project code I am working on?

This is what we are doing.

1. factory.getTx()
2. Do all db operations
3. commit at the end.
4. if exception, in the catch block we are calling manual rollback (commit is never called).
5. close graph in finally

Am I missing any step?

[wolf4ood]

hi @careerscale

i mean in the implementation of Tinkerpop3 of OrientDB.
http://tinkerpop.apache.org/docs/current/reference/#transactions
The default way would be like you described

1. factory.getTx()
2. Do all db operations
3. commit at the end.
4. if exception, in the catch block we are calling manual rollback (commit is never called).
5. close graph in finally

[wolf4ood]

Btw i've implemented the new ResultSet for working directly with

OrientVertex/OrientEdge please tell me if you encounter issues

orientechnologies/orientdb-gremlin#130

Thanks

[careerscale]

@maggiolo00 , yes that broke our code, so modifying it with OGremlinResultSet now. this will be the one that will remain right?

[careerscale]

@maggiolo00,
is there a issue, I can track for
implementation of Tinkerpop3 of OrientDB. ?

[wolf4ood]

hi @careerscale

for transaction configuration i've raised an issue here

orientechnologies/orientdb-gremlin#131

for OGremlinResultSet yes it will be the new api for handling the result set. I'm sorry for the inconvenience but we have to change it now, before the 3.0 reaches the API freeze in RC1

Since OGremlinResultSet it's just a wrapper for OResultSet, i've just pushed a method on
OGremlinResultSet#getRawResultSet where you can get the underlying OResultSet of OrientDB.
In this way you can change less code, and migrate to the new OGremlinResultSet incrementally

Thanks

[careerscale]

@maggiolo00 , Looks like DB is really unstable

[careerscale]

@maggiolo00 #7470 can you pleaes check this one?

[wolf4ood]

@careerscale

i'm checking it

[careerscale]

@maggiolo00 Thank you.

Please let me know if you need any inputs

[tglman]

hi @careerscale,

This should be already fixed in the last 3.0.0-SNAPSHOT, can we close this ?

Regards

[careerscale]

@tglman , do we need rollback stmt manually or is it automatic (if no commit no data gets committed?).

[careerscale]

we are using rollback everytime now, based on your confirmation, will remove rollback stmt

[tglman]

hi @careerscale,

Yes using TP3 API is correct to add a try{ ... }catch(...){ graph.rollback();}.

with TP3 as today we have automatic commit on close

Regards

[careerscale]

oh ok, so there is automatic commit but rollback is mandatory. got it.

does it hurt if we have commit stmt too?

Also if there are no data manipulations but only reads, does this rolllback hurt?

[tglman]

hi @careerscale,

If the transaction is already committed the close does nothing, so a commit in the code doesn't hurt.
If the are no changes in the transaction the commit does nothing, so a commit with no changes doesn't hurt.
If the rollback is called in case there are no changes or no active transactions, it does nothing, so it should not hurt.

Hope it helps.

Regards

[careerscale]

yes, that is good for me. thank you.

[careerscale]

@tglman ,

Based on earlier conversations, we wrote a comit method that loops through in case of failures and retries it (if applicable). we wrote this in a base class and using it across.

Do you suggest to retain this or remove it now? assuming this is done as part of the commit process (so the client api takes care of it automatically and application doesnt need to do it?).

        for (int retry = 0; retry < 10; ++retry) {
            try {
                graph.commit();
                break;
            } catch (ONeedRetryException e) {
                LOG.warn("ONeedRetryException : {}", e.getMessage());
                // IT'S OK, RETRY IT
            }
        }
    }

[tglman]

hi @careerscale,

The retry should be done on the whole transaction not just on the commit, so you can do retrying but this need to be wrapped around the whole tx operations.

[v-r-sankar]

I am using Tinkerpop API
Here is my code
OrientGraphFactory factory = new OrientGraphFactory(configuration)
List vertexList = new ArrayList(); // Added 100 vertices
threadpool.submit( () -> {
Graph graph = factory.getTx();
GraphTraversalSource gt = graph.traversal();
for (int x = 0; x < 5; x++) {
addRandomEdge(vertexList, gt);
}
for (int k = 0; k < 3; k++) {
try {
graph.tx().commit();
logger.info("Graph commited in iteration : {}", k);
break;
} catch (Exception e) {
if (k == 2) {
logger.error("commiting edge");
} else {
logger.warn("commit edge failed. Retrying : {}", k);
}
}
}
});

If the commit fails for the first time due to out of date vertices in the graph, graph.tx().commit is not succeeding again on retry.
What am I doing incorrectly? Could you suggest?