Skip to content
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

Add @UnitOfWork annotation #731

Closed
gissuebot opened this issue Jul 7, 2014 · 4 comments
Closed

Add @UnitOfWork annotation #731

gissuebot opened this issue Jul 7, 2014 · 4 comments

Comments

@gissuebot
Copy link

From spootsy.ootsy on October 05, 2012 00:28:48

Something that seems to be missing from guice-persist is a @UnitOfWork equivalent to the @Transactional annotation. One that will automatically start/stop a UnitOfWork as required, but not begin/commit a transaction.

Our use case: we have a bunch of classes that are used for database access. We use these in command-line applications, both during initialisation (in provider methods) and at run-time.

If a method exists for altering data in the database, we can happily apply @Transactional to it. It can now be safely called whether you're in an active UnitOfWork or not (because @Transactional will only start/stop a UnitOfWork if required).

For methods that only read data from the database, we don't want to start/commit transactions needlessly, so the @Transactional tag is a no-go. But - we have no means of guaranteeing that a method is encapsulated by a UnitOfWork automatically... there's no way to check if a UnitOfWork is already running, so there's no way to start/stop one only as required.

Would be a boon to have this functionality. Alternatively, an "isRunning()" method on UnitOfWork would make it possible for us (and others) to create our own automatic UnitOfWork controllers.

Original issue: http://code.google.com/p/google-guice/issues/detail?id=731

@gissuebot
Copy link
Author

From spootsy.ootsy on October 10, 2012 16:46:02

Patch attached to issue 730 which addresses this.

It doesn't expose the isWorking method, but rather introduces an invocation-counting mechanism to JpaPersistService so that begin()/end() calls can be nested safely.

@gissuebot
Copy link
Author

From julian.wyngaard on July 24, 2013 20:22:18

I haven't used the @UnitOfWork part of this patch, but I don't think the fix in JpaPersistService where it increments and decrements the count is working correctly.

I thrashed the service with soapUI using a load test on several service(all using PersistFilter) in my app simultaneously. I'm not sure how, since it's a ThreadLocal variable, but that count quickly get out of whack. As a result not all entity managers are getting closed and the connection pool eventually runs out of connections. Works fine with one thread.

Perhaps it's not handling nested @Transactions correctly. I wasted a lot of time trying get this patch working.

@gissuebot
Copy link
Author

From spootsy.ootsy on July 29, 2013 18:53:51

Hi @julian,

Thanks for trying out the patch. Sorry to hear you had problems with it.

Having had a look at the patch with fresh eyes, you hit the nail on the head. Nested transactions call "unitOfWork.begin()", but not "unitOfWork.end()", which would cause the problem you saw.

I'll post an updated patch when I get a chance. I've searched for any other possible mismatched begin/end calls, but that's the only one.

It should be an easy fix if you want to do it yourself. In JpaLocalTxnInterceptor, just add a try/finally block around line 56, so it becomes:

try {
  return methodInvocation.proceed();
} finally {
  unitOfWork.end();
}

Hope that helps some :)

@gissuebot
Copy link
Author

From sberlin on December 20, 2013 06:16:50

(No comment was entered for this change.)

Labels: Component-Persist

andresviedma added a commit to andresviedma/guice that referenced this issue Oct 25, 2015
andresviedma added a commit to andresviedma/guice that referenced this issue Oct 25, 2015
andresviedma added a commit to andresviedma/guice that referenced this issue Dec 31, 2015
andresviedma added a commit to andresviedma/guice that referenced this issue Dec 31, 2015
copybara-service bot pushed a commit that referenced this issue Apr 21, 2023
…because it leads to leaks. Fixes #739, fixes #997, fixes #730, fixes #985, fixes #959, fixes #731. This also adds an optional Options to the JpaPersistModule constructor, if users wish to use the legacy behavior. This is a breaking change, but to a better default value. Users who want to keep the dangerous form can opt back in with the options.

PiperOrigin-RevId: 525852009
copybara-service bot pushed a commit that referenced this issue Apr 21, 2023
…because it leads to leaks. Fixes #739, fixes #997, fixes #730, fixes #985, fixes #959, fixes #731. This also adds an optional Options to the JpaPersistModule constructor, if users wish to use the legacy behavior. This is a breaking change, but to a better default value. Users who want to keep the dangerous form can opt back in with the options.

PiperOrigin-RevId: 525852009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment