[jboss-jira] [JBoss JIRA] (DROOLS-3228) Rules don't produce the correct result anymore after a session reset

[ https://issues.jboss.org/browse/DROOLS-3228?page=com.atlassian.jira.plugi... ] Christian Liebhardt updated DROOLS-3228: ---------------------------------------- Description: Hello, A while ago I've created a test project to compare the different possibilities of session types and session polling/reusing. After an update to Drools 7.13.0.Final I found a performance improvement but also two issues. I would summarize both issues in this ticket as the second one is really minor. Let me know if you have a different preference. Also I'm adding code changes to the ticket description. If you like I can also create a pull request for that. The reason why I hesitated to do this right away is that I'm unclear regarding the solution of the first issue. *1. Incorrect results* If you clone the project and execute [StatefulDroolsEngineUnitTest.java|https://github.com/liebharc/JavaRules/b...] then you will see test failures with 7.13.0.Final while the tests pass with 7.12.0.Final. I could trace the issue back to a recent change in ConcurrentNodeMemories. If I revert this change on a snapshot build of 7.14.0 then the tests pass again. Here the patch for the revert: {code:java} diff --git a/drools-core/src/main/java/org/drools/core/common/ConcurrentNodeMemories.java b/drools-core/src/main/java/org/drools/core/common/ConcurrentNodeMemories.java index 020446190c..48045697af 100644 --- a/drools-core/src/main/java/org/drools/core/common/ConcurrentNodeMemories.java +++ b/drools-core/src/main/java/org/drools/core/common/ConcurrentNodeMemories.java @@ -16,6 +16,8 @@ package org.drools.core.common; +import java.util.HashSet; +import java.util.Set; import java.util.concurrent.atomic.AtomicReferenceArray; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -53,20 +55,24 @@ public class ConcurrentNodeMemories implements NodeMemories { public void resetAllMemories(StatefulKnowledgeSession session) { InternalKnowledgeBase kBase = (InternalKnowledgeBase)session.getKieBase(); + Set<SegmentMemory> smems = new HashSet<SegmentMemory>(); for (int i = 0; i < memories.length(); i++) { Memory memory = memories.get(i); if (memory != null) { if (memory.getSegmentMemory() != null) { - SegmentMemory smem = memory.getSegmentMemory(); - smem.reset(kBase.getSegmentPrototype(smem)); - if ( smem.isSegmentLinked() ) { - smem.notifyRuleLinkSegment((InternalWorkingMemory)session); - } + smems.add(memory.getSegmentMemory()); } memory.reset(); } } + + for (SegmentMemory smem : smems) { + smem.reset(kBase.getSegmentPrototype(smem)); + if ( smem.isSegmentLinked() ) { + smem.notifyRuleLinkSegment((InternalWorkingMemory)session); + } + } } /** {code} I tried to find out why the order matters here, but apparently lack knowledge. *2. Reset doesn't bring a disposed session back to life* With 7.12.0.Final it was possible to bring a disposed session back to life by calling the reset method. In 7.13.0.Final that doesn't work anymore because the checkAlive validation then fails. The new session pools seem to deal with that by just setting the alive flag back to true if a session is reused. While the new session pools are great, we would like to stick with our existing and simpler session pooling which makes use of the reset method. So would it be possible to set the alive flag to true on a session reset? {code:java} diff --git a/drools-core/src/main/java/org/drools/core/impl/StatefulKnowledgeSessionImpl.java b/drools-core/src/main/java/org/drools/core/impl/StatefulKnowledgeSessionImpl.java index d6f4959e0d..4a6199ad23 100644 --- a/drools-core/src/main/java/org/drools/core/impl/StatefulKnowledgeSessionImpl.java +++ b/drools-core/src/main/java/org/drools/core/impl/StatefulKnowledgeSessionImpl.java @@ -1122,6 +1122,8 @@ public class StatefulKnowledgeSessionImpl extends AbstractRuntime this.processRuntime = null; this.initialFactHandle = initInitialFact(kBase, null); + + this.alive = true; } public void reset(int handleId, {code} was: Hello, A while ago I've created a test projects to compare the different possibilities of session types and session polling/reusing. After an update to Drools 7.13.0.Final I found a performance improvement but also two issues. I would summarize both issues in this ticket as the second one is really minor. Let me know if you have a different preference. Also I'm adding code changes to the ticket description. If you like I can also create a pull request for that. The reason why I hesitated to do this right away is that I'm unclear regarding the solution of the first issue. *1. Incorrect results* If you clone the project and execute [StatefulDroolsEngineUnitTest.java|https://github.com/liebharc/JavaRules/b...] then you will see test failures with 7.13.0.Final while the tests pass with 7.12.0.Final. I could trace the issue back to a recent change in ConcurrentNodeMemories. If I revert this change on a snapshot build of 7.14.0 then the tests pass again. Here the patch for the revert: {code:java} diff --git a/drools-core/src/main/java/org/drools/core/common/ConcurrentNodeMemories.java b/drools-core/src/main/java/org/drools/core/common/ConcurrentNodeMemories.java index 020446190c..48045697af 100644 --- a/drools-core/src/main/java/org/drools/core/common/ConcurrentNodeMemories.java +++ b/drools-core/src/main/java/org/drools/core/common/ConcurrentNodeMemories.java @@ -16,6 +16,8 @@ package org.drools.core.common; +import java.util.HashSet; +import java.util.Set; import java.util.concurrent.atomic.AtomicReferenceArray; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -53,20 +55,24 @@ public class ConcurrentNodeMemories implements NodeMemories { public void resetAllMemories(StatefulKnowledgeSession session) { InternalKnowledgeBase kBase = (InternalKnowledgeBase)session.getKieBase(); + Set<SegmentMemory> smems = new HashSet<SegmentMemory>(); for (int i = 0; i < memories.length(); i++) { Memory memory = memories.get(i); if (memory != null) { if (memory.getSegmentMemory() != null) { - SegmentMemory smem = memory.getSegmentMemory(); - smem.reset(kBase.getSegmentPrototype(smem)); - if ( smem.isSegmentLinked() ) { - smem.notifyRuleLinkSegment((InternalWorkingMemory)session); - } + smems.add(memory.getSegmentMemory()); } memory.reset(); } } + + for (SegmentMemory smem : smems) { + smem.reset(kBase.getSegmentPrototype(smem)); + if ( smem.isSegmentLinked() ) { + smem.notifyRuleLinkSegment((InternalWorkingMemory)session); + } + } } /** {code} I tried to find out why the order matters here, but apparently lack knowledge. *2. Reset doesn't bring a disposed session back to life* With 7.12.0.Final it was possible to bring a disposed session back to life by calling the reset method. In 7.13.0.Final that doesn't work anymore because the checkAlive validation then fails. The new session pools seem to deal with that by just setting the alive flag back to true if a session is reused. While the new session pools are great, we would like to stick with our existing and simpler session pooling which makes use of the reset method. So would it be possible to set the alive flag to true on a session reset? {code:java} diff --git a/drools-core/src/main/java/org/drools/core/impl/StatefulKnowledgeSessionImpl.java b/drools-core/src/main/java/org/drools/core/impl/StatefulKnowledgeSessionImpl.java index d6f4959e0d..4a6199ad23 100644 --- a/drools-core/src/main/java/org/drools/core/impl/StatefulKnowledgeSessionImpl.java +++ b/drools-core/src/main/java/org/drools/core/impl/StatefulKnowledgeSessionImpl.java @@ -1122,6 +1122,8 @@ public class StatefulKnowledgeSessionImpl extends AbstractRuntime this.processRuntime = null; this.initialFactHandle = initInitialFact(kBase, null); + + this.alive = true; } public void reset(int handleId, {code} -- This message was sent by Atlassian Jira (v7.12.1#712002)