-
Notifications
You must be signed in to change notification settings - Fork 435
grpc services and @Transactional #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Have you tried to autowire the transactional bean into grpc service? |
If you mean wiring a transactional facade yes that's fine and that's how we
do it now. But the fact that the grpcservice is a @component and it's
methods look like the boundary methods of a traditional facade makes people
want to put annotations on them. At least that's what happened in our team
several times :-)
Op wo 24 mei 2017 23:53 schreef Furer Alexander <[email protected]>:
… Have you tried to autowire the transactional bean into grpc service?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#41 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAL1AFCooffTnIhOLE9DWvqRtcA4wT9Tks5r9KbCgaJpZM4Nkwba>
.
|
As I understand, the service method is being invoked by grpc framework directly and spring has no idea about it, that's why it can't create transaction around the invocation... I think custom grpc interceptor is the way to apply the @transactional logic.... |
Another thought : I was taught to always have clear separation of responsibility layers : transport, business and data access. Having @transactional being applied to transport method smells :-), at least for me. So, IMHO, you should continue to have facade to integrate with DB. What is your opinion? |
yes the grpc interceptor would suffice for the basic case, and custom per-method transactional semantics are more easily expressed using the separate transactional facade. About the layer responsibility, It's the accepted way in java to pretend a clean architecture :-) . I tend to agree but for the simple case layering just bogs things down. |
As a Spring framework user I confirm the natural expectation of @transactional (and other Spring goodness) working for grpc services, just like it works with e.g. spring MVC controllers and apache CXF services. |
@pagrus7 , contributions are welcome. |
@pagrus7 how do you see this working with streaming rpc's ? The tx interceptor will only work well for the standard "simple RPC" case, once the client starts streaming you can no longer assume the same model applies. In some cases you might want to start a transaction server side per streaming call, whereas others would want the whole stream to be in the same transaction. You would probably need to implement a custom annotation or attribute on @GrpcService to express all these semantics. Don't think this is a small feat :-) |
@jorgheymans I agree that in streaming context the declarative transactions on GRPC service are unlikely to be useful. Can't think of a valid use case. Unary RPC calls though can get the most benefit from transactions, and there are tons of use cases for them. In short, I see @transactional working exactly like it would elsewhere: transaction opens before the method execution and commits right after. It is up to developer to decide on the meaningful use with respect to the RPC pattern they choose. With all that said, based on a few experiments I suspect that transaction management (and other proxy-based goodness) does work already, assuming the right configuration is in place. Stay tuned as I'm writing the next comment. |
Here is my example which combines @transactional and @GRpcService. It configures spring for proxying target class rather than using dynamic proxies. Looks like this proxy mode will be default in Spring Boot 2.x. I encourage others to try switching from jdk proxies to cglib and validating if this indeed is a solution. Update: noticed too late that AOP with class proxies is already under test |
@pagrus7 interesting thanks, so it seems that at most this 'feature' would need a small section in the documentation explaining things - great :) |
It's interesting to see that interceptors are invoked before the actual proxied service call, run the
|
@jvmlet while I'm very new to GRPC, it looks like the interceptors are intended for augmenting a chain of listeners and handlers, rather than executing the cross-cutting logic in-place. E.g. the following implementation of a logging interceptor @Override
public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call, Metadata headers,
ServerCallHandler<ReqT, RespT> next) {
ForwardingServerCall.SimpleForwardingServerCall loggingCall = new ForwardingServerCall.SimpleForwardingServerCall(call) {
@Override
public void sendMessage(Object message) {
log.info("{} - responding with message - {} ", call.getMethodDescriptor().getFullMethodName(), message);
super.sendMessage(message);
}
};
ServerCall.Listener<ReqT> loggingListener = next.startCall(loggingCall, headers);
SimpleForwardingServerCallListener loggingListenerWrapper = new SimpleForwardingServerCallListener(loggingListener) {
@Override
public void onMessage(Object message) {
log.info("{} - received message - {} ", call.getMethodDescriptor().getFullMethodName(), message);
super.onMessage(message);
}
@Override
public void onComplete() {
log.info("{} - call completed", call.getMethodDescriptor().getFullMethodName());
super.onComplete();
}
};
return loggingListenerWrapper;
} ... produces the output which makes sense to me
|
I would expect the |
Added a one-liner PR for README.adoc which mentions class proxying. |
Still, if your interceptor requires transaction, it will not be available... Only the service method itself will be transactional... |
@jvmlet I think this is "business as usual". If an interceptor needs a transaction, it can start one. |
@jorgheymans, what do you think? Should I close this issue? |
Yes can be closed, thanks !
Op di 10 okt. 2017 19:19 schreef Furer Alexander <[email protected]>:
… @jorgheymans <https://github.com/jorgheymans>, what do you think? Should
I close this issue?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#41 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAL1AKFN77l5NgoKQ3Nz1XtffB-uZib0ks5sq6cagaJpZM4Nkwba>
.
|
Hi,
It seems that spring's @transactional has no effect when used in a @GrpcService class. This is probably expected but nevertheless surprising. Integrating with spring's tx interceptor mechanics could make a worthy addition to this starter.
(and no i don't have a PR ready)
The text was updated successfully, but these errors were encountered: