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.

Sign up for GitHub

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

[JBPM-10187] Sorting resources to avoid deadlock #2318

Merged
merged 6 commits into from
Nov 6, 2023
Merged

[JBPM-10187] Sorting resources to avoid deadlock #2318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
71acdb6
[JBPM-10187] Adding traces for process instance info
fjtirado Aug 17, 2023
6a613b9
[JBPM-10187] Propagate exceptions different than SessionNotFound
fjtirado Aug 25, 2023
2779e26
[JBPM-10187] Handling sessionnotfound in local thread
fjtirado Sep 18, 2023
3247732
[JBPM-10187] Using proper SessionNotFoundException
fjtirado Sep 25, 2023
e3c32e8
[JBPM-10187] Fully logging marshaling exception
fjtirado Oct 2, 2023
f21a8d7
[JBPM-10187] Gonzalo comments
fjtirado Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ...jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/JpaProcessPersistenceContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
public class JpaProcessPersistenceContext extends JpaPersistenceContext
implements
ProcessPersistenceContext {


public JpaProcessPersistenceContext(EntityManager em, TransactionManager txm) {
super( em, txm );
Expand All @@ -66,7 +67,10 @@ public PersistentProcessInstance findProcessInstanceInfo(Long processId) {
if( this.pessimisticLocking ) {
return em.find( ProcessInstanceInfo.class, processId, lockMode );
}
return em.find( ProcessInstanceInfo.class, processId );
logger.trace("Reading process instance info {} with em {}", processId, em);
ProcessInstanceInfo pi = em.find(ProcessInstanceInfo.class, processId);
logger.trace("Process instance info read {}", pi);
fjtirado marked this conversation as resolved.
Show resolved Hide resolved
return pi;
}

public void remove(PersistentProcessInstance processInstanceInfo) {
Expand Down
6 changes: 6 additions & 0 deletions ...rsistence-jpa/src/main/java/org/jbpm/persistence/processinstance/ProcessInstanceInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,14 @@
import org.jbpm.workflow.instance.impl.WorkflowProcessInstanceImpl;
import org.kie.api.runtime.Environment;
import org.kie.api.runtime.process.ProcessInstance;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@Entity
@SequenceGenerator(name="processInstanceInfoIdSeq", sequenceName="PROCESS_INSTANCE_INFO_ID_SEQ")
public class ProcessInstanceInfo implements PersistentProcessInstance {

private static final Logger logger = LoggerFactory.getLogger(ProcessInstanceInfo.class);
@Id
@GeneratedValue(strategy = GenerationType.AUTO, generator="processInstanceInfoIdSeq")
@Column(name = "InstanceId")
Expand Down Expand Up @@ -193,6 +196,9 @@ public ProcessInstance getProcessInstance(InternalKnowledgeRuntime kruntime,
context.close();
} catch ( IOException e ) {
throw new IllegalArgumentException( "IOException while loading process instance: " + e.getMessage(), e);
} catch (RuntimeException e) {
logger.error("Error unmarshalling process instance info {}", processInstanceId, e);
throw e;
}
}
((WorkflowProcessInstanceImpl) processInstance).internalSetStartDate(this.startDate);
Expand Down
5 changes: 3 additions & 2 deletions jbpm-runtime-manager/src/main/java/org/jbpm/runtime/manager/impl/AbstractRuntimeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
import org.kie.internal.runtime.manager.RuntimeManagerRegistry;
import org.kie.internal.runtime.manager.SecurityManager;
import org.kie.internal.runtime.manager.SessionFactory;
import org.kie.internal.runtime.manager.SessionNotFoundException;
import org.kie.internal.runtime.manager.TaskServiceFactory;
import org.kie.internal.runtime.manager.context.ProcessInstanceIdContext;
import org.kie.internal.runtime.manager.deploy.DeploymentDescriptorManager;
Expand Down Expand Up @@ -389,8 +390,8 @@ protected boolean canDispose(RuntimeEngine runtime) {
&& tm.getStatus() != TransactionManager.STATUS_COMMITTED) {
return false;
}
} catch (Exception e) {
logger.warn("Exception dealing with transaction", e);
} catch (SessionNotFoundException e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not equivalent. lossing info in other exceptions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention of this change is to let the transaction fail if the exception is different from SessionNotFoundException, but to be honest I do not recall why I though that will be useful for this particular issue.
So, I guess the question is, does it make sense to hide all exceptions here or just the SesisonNotFound?

logger.warn("Session not found exception", e);
}
return true;
}
Expand Down
37 changes: 33 additions & 4 deletions ...manager/src/main/java/org/jbpm/runtime/manager/impl/PerProcessInstanceRuntimeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@
package org.jbpm.runtime.manager.impl;

import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;

import org.drools.core.command.SingleSessionCommandService;
import org.drools.core.command.impl.CommandBasedStatefulKnowledgeSession;
Expand Down Expand Up @@ -191,15 +194,41 @@ public void signalEvent(String type, Object event) {

// process currently active runtime engines
Map<Object, RuntimeEngine> currentlyActive = local.get();

if (currentlyActive != null && !currentlyActive.isEmpty()) {
RuntimeEngine[] activeEngines = currentlyActive.values().toArray(new RuntimeEngine[currentlyActive.size()]);
for (RuntimeEngine engine : activeEngines) {
Context<?> context = ((RuntimeEngineImpl) engine).getContext();
@SuppressWarnings("unchecked")
Entry<Object, RuntimeEngine> activeEngines[] = currentlyActive.entrySet()
.toArray(new Entry[currentlyActive.size()]);
fjtirado marked this conversation as resolved.
Show resolved Hide resolved
Set<Object> enginesToDelete = new HashSet<>();
for (Entry<Object, RuntimeEngine> engine : activeEngines) {
fjtirado marked this conversation as resolved.
Show resolved Hide resolved
RuntimeEngineImpl engineImpl = (RuntimeEngineImpl) engine.getValue();
if (engineImpl==null) {
continue;
}
if (engineImpl.isDisposed() || engineImpl.isInvalid()) {
fjtirado marked this conversation as resolved.
Show resolved Hide resolved
Object engineKey = engine.getKey();
logger.trace("Engine with key {} is not longer valid", engineKey);
enginesToDelete.add(engineKey);
continue;
}
Context<?> context = engineImpl.getContext();
if (context != null && context instanceof ProcessInstanceIdContext
&& ((ProcessInstanceIdContext) context).getContextId() != null) {
engine.getKieSession().signalEvent(type, event, ((ProcessInstanceIdContext) context).getContextId());
try {
engineImpl.getKieSession().signalEvent(type, event,
((ProcessInstanceIdContext) context).getContextId());
} catch (org.drools.persistence.api.SessionNotFoundException ex) {
logger.warn(
"Signal event cannot proceed because of session not found exception {} for engine {}",
ex.getMessage(), engineImpl.getKieSessionId());
enginesToDelete.add(engine.getKey());
}
}
}
if (!enginesToDelete.isEmpty()) {

currentlyActive.keySet().removeAll(enginesToDelete);
fjtirado marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

Expand Down
Loading