This is an automated email from the ASF dual-hosted git repository.
mariofusco pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-kie-drools.git
The following commit(s) were added to refs/heads/main by this push:
new 14f5601b46 [KIE-DROOLS-6190] fix removal of detached tuples during
incremental compilation (#6192)
14f5601b46 is described below
commit 14f5601b464965e50141fef98ba67a16de6886c5
Author: Mario Fusco <[email protected]>
AuthorDate: Fri Dec 13 10:56:43 2024 +0100
[KIE-DROOLS-6190] fix removal of detached tuples during incremental
compilation (#6192)
* [KIE-DROOLS-6190] fix removal of detached tuples during incremental
compilation
* add missing license
* wip
* wip
* wip
---
.../org/drools/core/common/DefaultFactHandle.java | 28 +------
.../org/drools/core/phreak/EagerPhreakBuilder.java | 23 +-----
.../drools/core/phreak/RuleNetworkEvaluator.java | 2 +-
.../java/org/drools/core/reteoo/ReteooBuilder.java | 4 +-
.../AddRemoveGenerated2RulesEvalTest.java | 1 -
.../AddRemoveGenerated2RulesIntegerTest.java | 1 -
.../AddRemoveGenerated2RulesMapContainsTest.java | 1 -
.../AddRemoveGenerated2RulesNotNotTest.java | 1 -
.../AddRemoveGenerated2RulesNotTest.java | 1 -
.../AddRemoveGenerated2RulesStringIntegerTest.java | 1 -
.../AddRemoveGenerated2RulesStringTest.java | 1 -
.../CasesFromGeneratedRulesTest.java | 87 ++++++++++++++++++++++
12 files changed, 93 insertions(+), 58 deletions(-)
diff --git
a/drools-core/src/main/java/org/drools/core/common/DefaultFactHandle.java
b/drools-core/src/main/java/org/drools/core/common/DefaultFactHandle.java
index 51365d30d7..88a05d6e05 100644
--- a/drools-core/src/main/java/org/drools/core/common/DefaultFactHandle.java
+++ b/drools-core/src/main/java/org/drools/core/common/DefaultFactHandle.java
@@ -507,30 +507,6 @@ public class DefaultFactHandle extends
AbstractLinkedListNode<DefaultFactHandle>
lastLeftTuple = leftTuple;
}
- private void addLastTuple(TupleImpl tuple, boolean left) {
- if (left) {
- addLastLeftTuple(tuple);
- } else {
- addLastRightTuple(tuple);
- }
- }
-
- private void setFirstTuple(TupleImpl tuple, boolean left) {
- if (left) {
- firstLeftTuple = tuple;
- } else {
- firstRightTuple = tuple;
- }
- }
-
- private void setLastTuple(TupleImpl tuple, boolean left) {
- if (left) {
- lastLeftTuple = tuple;
- } else {
- lastRightTuple = tuple;
- }
- }
-
@Override
public void removeLeftTuple( TupleImpl leftTuple ) {
TupleImpl previous = leftTuple.getHandlePrevious();
@@ -683,6 +659,7 @@ public class DefaultFactHandle extends
AbstractLinkedListNode<DefaultFactHandle>
if (detached != null) {
if (firstLeftTuple == detached) {
firstLeftTuple = null;
+ lastLeftTuple = null;
}
if (lastLeftTuple == detached) {
@@ -695,7 +672,6 @@ public class DefaultFactHandle extends
AbstractLinkedListNode<DefaultFactHandle>
lastLeftTuple.setHandleNext(null);
}
}
-
return detached;
}
@@ -711,6 +687,7 @@ public class DefaultFactHandle extends
AbstractLinkedListNode<DefaultFactHandle>
if (detached != null) {
if (firstRightTuple == detached) {
firstRightTuple = null;
+ lastRightTuple = null;
}
if (lastRightTuple == detached) {
@@ -723,7 +700,6 @@ public class DefaultFactHandle extends
AbstractLinkedListNode<DefaultFactHandle>
lastRightTuple.setHandleNext(null);
}
}
-
return detached;
}
diff --git
a/drools-core/src/main/java/org/drools/core/phreak/EagerPhreakBuilder.java
b/drools-core/src/main/java/org/drools/core/phreak/EagerPhreakBuilder.java
index 2c4e58d806..15478b1a2b 100644
--- a/drools-core/src/main/java/org/drools/core/phreak/EagerPhreakBuilder.java
+++ b/drools-core/src/main/java/org/drools/core/phreak/EagerPhreakBuilder.java
@@ -151,8 +151,6 @@ public class EagerPhreakBuilder implements PhreakBuilder {
}
}
- Set<SegmentMemoryPair> smemsToNotify = new HashSet<>();
-
if (exclBranchRoots.isEmpty()) {
LeftTupleNode lian = tn.getPathNodes()[0];
processLeftTuples(lian, false, tn, wms);
@@ -163,7 +161,7 @@ public class EagerPhreakBuilder implements PhreakBuilder {
// Process existing branches from the split points
Set<Integer> visited = new HashSet<>();
- exclBranchRoots.forEach(pair -> Remove.processMerges(pair.parent,
tn, kBase, wms, visited, smemsToNotify));
+ exclBranchRoots.forEach(pair -> Remove.processMerges(pair.parent,
tn, kBase, wms, visited));
}
for (InternalWorkingMemory wm : wms) {
@@ -174,8 +172,6 @@ public class EagerPhreakBuilder implements PhreakBuilder {
pmem.getRuleAgendaItem().dequeue();
}
}
-
- smemsToNotify.forEach(pair -> pair.sm.notifyRuleLinkSegment(pair.wm));
}
public static void notifyImpactedSegments(SegmentMemory smem,
InternalWorkingMemory wm, Set<SegmentMemoryPair> segmentsToNotify) {
@@ -736,8 +732,7 @@ public class EagerPhreakBuilder implements PhreakBuilder {
}
}
-
- private static void processMerges(LeftTupleNode splitNode,
TerminalNode tn, InternalRuleBase kBase, Collection<InternalWorkingMemory> wms,
Set<Integer> visited, Set<SegmentMemoryPair> smemsToNotify) {
+ private static void processMerges(LeftTupleNode splitNode,
TerminalNode tn, InternalRuleBase kBase, Collection<InternalWorkingMemory> wms,
Set<Integer> visited) {
// it's possible for a rule to have multiple exclBranches,
pointing to the same parent. So need to ensure it's processed once.
if ( !visited.add(splitNode.getId())) {
return;
@@ -766,10 +761,7 @@ public class EagerPhreakBuilder implements PhreakBuilder {
}
SegmentPrototype proto2 = kBase.getSegmentPrototype(ltn);
-
mergeSegments(proto1, proto2, kBase, wms);
-
- notifyImpactedSegments(wms, proto1, smemsToNotify);
}
}
@@ -1443,17 +1435,6 @@ public class EagerPhreakBuilder implements PhreakBuilder
{
}
}
- private static void
notifyImpactedSegments(Collection<InternalWorkingMemory> wms, SegmentPrototype
proto1, Set<SegmentMemoryPair> smemsToNotify) {
- // any impacted segments must be notified for potential linking
- for (InternalWorkingMemory wm : wms) {
- Memory mem1 =
wm.getNodeMemories().peekNodeMemory(proto1.getRootNode());
- if (mem1 != null && mem1.getSegmentMemory() != null) {
- // there was a split segment, both need notifying.
- notifyImpactedSegments(mem1.getSegmentMemory(), wm,
smemsToNotify);
- }
- }
- }
-
private static void setNodeTypes(SegmentPrototype proto, LeftTupleNode[]
protoNodes) {
int nodeTypesInSegment = 0;
for ( LeftTupleNode node : protoNodes) {
diff --git
a/drools-core/src/main/java/org/drools/core/phreak/RuleNetworkEvaluator.java
b/drools-core/src/main/java/org/drools/core/phreak/RuleNetworkEvaluator.java
index eef6b8ad1b..8e8c12b4c4 100644
--- a/drools-core/src/main/java/org/drools/core/phreak/RuleNetworkEvaluator.java
+++ b/drools-core/src/main/java/org/drools/core/phreak/RuleNetworkEvaluator.java
@@ -273,7 +273,7 @@ public class RuleNetworkEvaluator {
}
boolean emptySrcTuples = srcTuples.isEmpty();
- if ( !(NodeTypeEnums.isBetaNode(node) &&
((BetaNode)node).isRightInputIsRiaNode() ) ) {
+ if ( !(NodeTypeEnums.isBetaNode(node) &&
node.isRightInputIsRiaNode() ) ) {
// The engine cannot skip a ria node, as the dirty might be
several levels deep
if ( emptySrcTuples && smem.getDirtyNodeMask() == 0) {
// empty sources and segment is not dirty, skip to non
empty src tuples or dirty segment.
diff --git
a/drools-core/src/main/java/org/drools/core/reteoo/ReteooBuilder.java
b/drools-core/src/main/java/org/drools/core/reteoo/ReteooBuilder.java
index 2d7ada53d3..4300fca8ce 100644
--- a/drools-core/src/main/java/org/drools/core/reteoo/ReteooBuilder.java
+++ b/drools-core/src/main/java/org/drools/core/reteoo/ReteooBuilder.java
@@ -193,9 +193,7 @@ public class ReteooBuilder
tn.visitLeftTupleNodes(n -> n.removeAssociatedTerminal(tn));
- BaseNode node = (BaseNode) tn;
- removeNodeAssociation(node, context.getRule(), new HashSet<>(),
context);
-
+ removeNodeAssociation((BaseNode) tn, context.getRule(), new
HashSet<>(), context);
resetMasks(removeNodes((AbstractTerminalNode)tn, workingMemories,
context));
}
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesEvalTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesEvalTest.java
index 52875fb88b..9470289315 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesEvalTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesEvalTest.java
@@ -22,7 +22,6 @@ import java.util.stream.Stream;
import org.junit.jupiter.api.Disabled;
-@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesEvalTest extends
AbstractAddRemoveGenerated2RulesTest {
public static Stream<ConstraintsPair> parameters() {
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesIntegerTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesIntegerTest.java
index 4c88cf1b0e..2ed797c431 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesIntegerTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesIntegerTest.java
@@ -22,7 +22,6 @@ import java.util.stream.Stream;
import org.junit.jupiter.api.Disabled;
-@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesIntegerTest extends
AbstractAddRemoveGenerated2RulesTest {
public static Stream<ConstraintsPair> parameters() {
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesMapContainsTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesMapContainsTest.java
index d849893844..f2188f23da 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesMapContainsTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesMapContainsTest.java
@@ -22,7 +22,6 @@ import java.util.stream.Stream;
import org.junit.jupiter.api.Disabled;
-@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesMapContainsTest extends
AbstractAddRemoveGenerated2RulesTest {
public static Stream<ConstraintsPair> parameters() {
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesNotNotTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesNotNotTest.java
index 77fae44b93..161c91cd72 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesNotNotTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesNotNotTest.java
@@ -22,7 +22,6 @@ import java.util.stream.Stream;
import org.junit.jupiter.api.Disabled;
-@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesNotNotTest extends
AbstractAddRemoveGenerated2RulesTest {
public static Stream<ConstraintsPair> parameters() {
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesNotTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesNotTest.java
index 8e8a40e759..e83554563b 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesNotTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesNotTest.java
@@ -22,7 +22,6 @@ import java.util.stream.Stream;
import org.junit.jupiter.api.Disabled;
-@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesNotTest extends
AbstractAddRemoveGenerated2RulesTest {
public static Stream<ConstraintsPair> parameters() {
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesStringIntegerTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesStringIntegerTest.java
index 9d2d3bdd8f..e2b49a6013 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesStringIntegerTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesStringIntegerTest.java
@@ -22,7 +22,6 @@ import java.util.stream.Stream;
import org.junit.jupiter.api.Disabled;
-@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesStringIntegerTest extends
AbstractAddRemoveGenerated2RulesTest {
public static Stream<ConstraintsPair> parameters() {
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesStringTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesStringTest.java
index fd97191f78..28bc702b4d 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesStringTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/AddRemoveGenerated2RulesStringTest.java
@@ -22,7 +22,6 @@ import java.util.stream.Stream;
import org.junit.jupiter.api.Disabled;
-@Disabled("It gets stuck. See issue #6190")
public class AddRemoveGenerated2RulesStringTest extends
AbstractAddRemoveGenerated2RulesTest {
public static Stream<ConstraintsPair> parameters() {
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/CasesFromGeneratedRulesTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/CasesFromGeneratedRulesTest.java
new file mode 100644
index 0000000000..13696bba1a
--- /dev/null
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/incrementalcompilation/CasesFromGeneratedRulesTest.java
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.compiler.integrationtests.incrementalcompilation;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+public class CasesFromGeneratedRulesTest {
+
+ @Test
+ @Timeout(40000)
+ public void
testInsertFactsFireRulesRemoveRulesReinsertRulesRevertedRules() {
+ String rule1 = """
+ package com.rules;
+ global java.util.List list
+ rule R1
+ when
+ Integer()
+ then
+ list.add('R1');
+ end
+ """;
+
+ String rule2 = """
+ package com.rules;
+ global java.util.List list
+ rule R2
+ when
+ Integer()
+ exists(Integer() and Integer())
+ exists(Integer() and Integer())
+ then
+ list.add('R2');
+ end
+ """;
+
+
AddRemoveTestCases.insertFactsFireRulesRemoveRulesReinsertRules1(rule1, rule2,
TestUtil.RULE1_NAME, TestUtil.RULE2_NAME, null, 1, 2);
+ }
+
+ @Test
+ @Timeout(40000)
+ public void testInsertFactsRemoveRulesFireRulesRemoveRules() {
+ String rule1 = """
+ package com.rules;
+ global java.util.List list
+ rule R1
+ when
+ exists(Integer())
+ not(Double() and Double())
+ Integer() not(Double() and Double())
+ then
+ list.add('R1');
+ end
+ """;
+
+ String rule2 = """
+ package com.rules;
+ global java.util.List list
+ rule R2
+ when
+ exists(Integer())
+ not(Double() and Double())
+ exists(Integer())
+ then
+ list.add('R2');
+ end
+ """;
+
+ AddRemoveTestCases.insertFactsRemoveRulesFireRulesRemoveRules2(rule1,
rule2, TestUtil.RULE1_NAME, TestUtil.RULE2_NAME, null, 1);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
