Transactional methods close before transaction is commited

[CleverUnderDog]

I have already found some discussion around @transactional in #41. Now I stumbled across an issue in my integration tests where the transaction is not committed yet but the gRPC call already returned. I'd like to help to contribute to this issue but I am missing some understanding of both the gRPC lifecycle of calls and the mechanism of dynamic proxies in Spring. Below an example of such a unit/integration test.

@Transactional
public void rpcCall(Req request, StreamOvserver<Res> observer) {
    // Database operations via JPARepositories
    observer.onNext(response);
    observer.onCompleted();
}

@Test
public void unitTest() {
    stub.rpcCall(request);

    // DB check if data is written.. sometimes changes are not yet committed in DB
}

My potential solution to this issue is to simple delay the calls to onNext, onError and onCompleted after the transaction is committed. This solution would only work for non streaming rpc calls. Any thoughts about this one?

[jvmlet]

Have you enabled proxy as described in https://github.com/LogNet/grpc-spring-boot-starter/pull/58/files?

[CleverUnderDog]

This enabled by default in the version of Spring boot that I am using. I can verify in stack trace that the method call is proxied. I can also use lazy loaded entries in received entities.

[CleverUnderDog]

Basically this represents a race condition between the database query in the unit test and the transaction handling of the proxy. I suspect the root cause for my inconsistent test result is the following order of execution.

UnitTest: request
Server:   gRPC gets call
Server:   Spring proxy open trx
Server:   rpc implementation (doing DB stuff)
Server:   onCompleted
UnitTest: Query Entity from DB
UnitTest: Old/No data received
Server:   close trx

[jvmlet]

OK, so the problem is that transactional annotation works around the method execution and the transaction is committed after the method completes and client's callback is invoked while the method is still being executed, triggered by onComplete...
I would prefer to setup the same single threaded thread pool for both grpc server and client for transactional testing rather than deferring client execution for some awkward value. I hope there will no be deadlock as grpc is NIO operated.

[CleverUnderDog]

I think this issue is not limited to unit tests. It could also be a problem in production scenarios. Imagine the client adds an entity to the database and then requests a list of all entities including the newly created one. The probability for being an issue is pretty low considering network latency. But unfortunately nothing is perfect and the database could need more time to successfully commit the transaction.

But you have a good point that deferring execution of onCompleted is probably not a good idea. In order to handle transactional methods it there needs to be a mechanism to separate the processing part from returning the result/error.

[jvmlet]

OK, in this case I would suggest to extract the transactional business logic into separate service bean method, mark it as transactional, autowire it into grpc service and call its method from grpc.

[jvmlet]

This is the same approach I suggested in #41 from the beginning

[CleverUnderDog]

Is it possible to archive this with some spring or grpc magic? I imagine it should be possible to proxy calls of onCompleted and onError.

[jvmlet]

IMHO, externalizing the transactional logic into separate service is the fastest and most reliable approach.

[CleverUnderDog]

This is exactly what I did. It's best to leave this as implementation detail in the business logic. It would be hard to handle streaming API in a generic way.

[micheljung]

IMHO, externalizing the transactional logic into separate service is the fastest and most reliable approach.

This only works if you have the result in memory, it doesn't if you stream from the database. See grpc/grpc-java#4694 (comment)

[jvmlet]

@micheljung , good catch, I suppose we should ask spring team to support @Transactional on method that returns Stream/Observable to commit transaction onClose/Unsubscribe rather than when method returns ;-)