DROOLS-1393 - Investigate why JpaOptLockPersistentStatefulSessionTest… #2019
Conversation
mswiderski
force-pushed
the
DROOLS-1393
branch
from
94b2410 to
550f481
Compare
|
@MarianMacik PR updated with your additional test |
mswiderski
force-pushed
the
DROOLS-1393
branch
from
550f481 to
5cdef64
Compare
|
Jenkins retest this |
|
Jenkins execute full downstream build |
mswiderski
force-pushed
the
DROOLS-1393
branch
from
5cdef64 to
d14fa31
Compare
|
@baldimir @MarianMacik made additional updates as the previous one were not enough, please have a look |
baldimir
left a comment
baldimir
left a comment
There was a problem hiding this comment.
Sorry for later review, few new comments.
| public void afterCompletion(int status) { | ||
| if ( status != TransactionManager.STATUS_COMMITTED ) { | ||
| this.service.rollback(); | ||
|
|
There was a problem hiding this comment.
Nitpicking - unnecessary empty line.
|
|
||
| public void resetApplicationScopedPersistenceContext() { | ||
| if ( this.internalAppScopedEntityManagerFlag ) { | ||
| if ( this.appScopedEntityManager != null && this.appScopedEntityManager.isOpen() ) { |
| new InsertAndFireThread(ksession.getIdentifier(), kbase, list).start(); | ||
| Thread thread = new InsertAndFireThread(ksession.getIdentifier(), kbase, list); | ||
| threads.add(thread); | ||
| thread.start(); |
There was a problem hiding this comment.
On a second thought, I think we need to use an Executor with proper shutdown here, so we know that the threads ended.
There was a problem hiding this comment.
We know this, simply because we are waiting for them on the line 114.
There was a problem hiding this comment.
OK after discussion it can stay as it is.
| executor.execute(() -> { | ||
| try { | ||
| ksession1.insert(1); | ||
| ksession1.dispose(); |
There was a problem hiding this comment.
Please dispose the session also in a try-finally, so we are really sure it got disposed. It could be a large try-finally that covers most of this test and at the end of the test disposes everything disposable.
| ksession2.insert(2); | ||
| ksession1latch.countDown(); | ||
| } | ||
| ksession2.dispose(); |
There was a problem hiding this comment.
Same as previous comment about dispose. I found out that when something bad happens to a test (could be caused by environment) and the session doesn't get disposed, if there is a Timer in the session, then it stays running till the whole test suite ends. That is why I am very cautious about proper session dispose.
mswiderski
force-pushed
the
DROOLS-1393
branch
from
d14fa31 to
35c7ede
Compare
|
@baldimir changes applied |
|
Jenkins execute full downstream build |
|
SLA tests are unstable on KIE Jenkins so I think their fails are unrelated, but I am worried about migration tests (migration report was null in these tests), though it can be unrelated as well. |
|
@MarianMacik migration related tests should be already fixed by |
|
jenkins retest this |
|
Jenkins retest this |
… is failing with Hibernate 5
mswiderski
force-pushed
the
DROOLS-1393
branch
from
35c7ede to
7a99f15
Compare
|
Jenkins execute full downstream build |
|
Looks good now (only unstable tests failed). We are good to merge. |
3 participants
… is failing with Hibernate 5