Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 1.10.5 (Hopper SR5)
-
Fix Version/s: 1.11 RC1 (Ingalls), 2.0 M2 (Kay)
-
Component/s: None
-
Labels:
-
Environment:Mac OS X Sierra 10.12.1
Description
I'm using Stream's to loop through each element in a Repository in an @Scheduled method. After running 200 times I get the following error:
2016-12-13 09:40:39.159 WARN 51004 --- [pool-2-thread-1] o.h.engine.jdbc.spi.SqlExceptionHelper : SQL Error: 0, SQLState: null |
2016-12-13 09:40:39.159 ERROR 51004 --- [pool-2-thread-1] o.h.engine.jdbc.spi.SqlExceptionHelper : [pool-2-thread-1] Timeout: Pool empty. Unable to fetch a connection in 2 seconds, none available[size:200; busy:200; idle:0; lastwait:2000]. |
The Repository is defined as such:
public interface PushNotificationRepository extends JpaRepository<PushNotification, Long> { |
@Query(value = "select p from PushNotification p") |
Stream<PushNotification> findAllAndStream();
|
}
|
The Service is defined like this:
@Service
|
@Transactional() |
public class PushNotificationFlusherService { |
private static final Logger LOGGER = LoggerFactory.getLogger(PushNotificationFlusherService.class); |
|
|
private final PushNotificationRepository pushNotificationRepository; |
|
|
@Autowired |
public PushNotificationFlusherService(PushNotificationRepository pushNotificationRepository) { |
this.pushNotificationRepository = pushNotificationRepository; |
}
|
|
|
@Scheduled(fixedDelay = 5000) |
protected void flushPushNotificationQueue() { |
LOGGER.info("Flushing push notifications"); |
|
|
try (Stream<PushNotification> pushNotifications = pushNotificationRepository.findAllAndStream()) { |
pushNotifications.forEach((PushNotification pushNotification) -> {
|
// Send notification |
});
|
}
|
|
|
LOGGER.info("Flushed push notifications"); |
}
|
I've tried using @Transactional( readOnly=true) but it does not make a difference.
Issue Links
- depends on
-
DATACMNS-959
Register repository interceptor to allow detecting a surrounding transaction
-
- Closed
-
- is duplicated by
-
DATAJPA-989
CrudRepository; return Stream<T>; The object is already closed [90007-192]; could not advance using next()
-
- Resolved
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Thank you @OlivierGierke, making the method public did the trick.
Thanks again for your lighting fast response 
Maarten Hus
added a comment - Thank you @OlivierGierke, making the method public did the trick.
Thanks again for your lighting fast response No worries, glad we got it working for you. We're currently investigating to even throw an exception in case we detect the transactions not being set up correctly when executing a method using Stream as a return type. I'll keep this open and potentially commit that fix against this ticket here.
Oliver Gierke
added a comment - No worries, glad we got it working for you. We're currently investigating to even throw an exception in case we detect the transactions not being set up correctly when executing a method using Stream as a return type. I'll keep this open and potentially commit that fix against this ticket here. This is now in place. A stream execution now checks whether there has been a transaction in place before the repository was invoked. If that's not the case, we reject the execution and give an indication why this is happening. This should give you a better idea of why things are not working in case you accidentally run into some misconfiguration like the one shown above.
Oliver Gierke
added a comment - This is now in place. A stream execution now checks whether there has been a transaction in place before the repository was invoked. If that's not the case, we reject the execution and give an indication why this is happening. This should give you a better idea of why things are not working in case you accidentally run into some misconfiguration like the one shown above. It's good to have this in place.
Oliver. Can you clarify the original behaviour and limitations of the new behaviour (I thought it's best to add it here as this is where we end up from the release notes).
The original behaviour seems to be a connection leakage either in Data JPA or in the underlying JPA provider's streaming support.
- Was there not an option to release the connection once the end of the result set was reached?
- Are we limited in having to wrap in a transaction or can we also add the OpenEntityManagerInViewFilter to allow us to stream all the way to the HttpResponse output stream?
- Looking at OpenEntityManagerInViewFilter it appears that async is handled too, so perhaps this is okay and perhaps a good example is all that's needed.
Neale Upstone
added a comment - It's good to have this in place.
Oliver. Can you clarify the original behaviour and limitations of the new behaviour (I thought it's best to add it here as this is where we end up from the release notes).
The original behaviour seems to be a connection leakage either in Data JPA or in the underlying JPA provider's streaming support.
Was there not an option to release the connection once the end of the result set was reached?
Are we limited in having to wrap in a transaction or can we also add the OpenEntityManagerInViewFilter to allow us to stream all the way to the HttpResponse output stream?
Looking at OpenEntityManagerInViewFilter it appears that async is handled too, so perhaps this is okay and perhaps a good example is all that's needed.
Do you run transactions in AspectJ mode? If not, adding @Transactional doesn't have any effect for protected methods. Making the method public (and maybe the entire service class package protected to avoid general visibility) should do the trick here.