Netflix / conductor

Conductor is a microservices orchestration engine.
Apache License 2.0
12.82k stars 2.34k forks source link

RedisExecutionDAO removeWorkflow method not remove WORKFLOW_TO_TASKS, cause memory leak #3153

Closed cuixiushuai closed 1 year ago

cuixiushuai commented 2 years ago

Describe the bug 1 jvm frequently full gc 2 dump the heap, com.netflix.conductor.jedis.JedisMock is the biggest object, on the setCache of org.rarefiedredis.redis.RedisMock, many HashNode, key like test.WORKFLOW_TO_TASKS.WorkflowInstanceId, value is empty set.

Details Conductor version: 2.27.6

Persistence implementation: JedisMock Workflow definition: Task definition: { "name": "test", "tasks": [ { "name": "findName", "type": "MY_SYSTEM_TASK", "inputParameters": { "name": "${workflow.input.header.name}" } } ] } Event handler definition:

To Reproduce Steps to reproduce the behavior:

  1. Go to 'com.netflix.conductor.dao.dynomite.RedisExecutionDAO#removeWorkflow'
  2. remove all the task from the workflow tasks set, but not remove the key (NAMESPACE_SEP.WORKFLOW_TO_TASKS.WorkflowInstanceId) 3 in the end, every time workflowExecutor.startWorkflow() and the workflow complete,the size of org.rarefiedredis.redis.RedisMock.cacheSet add one
cuixiushuai commented 2 years ago

hlep wanted, or need another info?

jxu-nflx commented 2 years ago

Hello @cuixiushuai I do see that mapping is removed by this line here: https://github.com/Netflix/conductor/blob/478ea92b169ddc688ad58d8693dc1837d7e8320f/redis-persistence/src/main/java/com/netflix/conductor/redis/dao/RedisExecutionDAO.java#L317

cuixiushuai commented 2 years ago

Hello @cuixiushuai I do see that mapping is removed by this line here:

https://github.com/Netflix/conductor/blob/478ea92b169ddc688ad58d8693dc1837d7e8320f/redis-persistence/src/main/java/com/netflix/conductor/redis/dao/RedisExecutionDAO.java#L317

on the setCache of RedisMock, memory workflow task like this:(WorkflowInstanceId) -> Set(taskId1,taskId2...); this line srem the mapping, just remove the task on the Set, at last the Set is cleard, but the mapping (WorkflowInstanceId) -> Set() is not removed

aravindanr commented 2 years ago

@cuixiushuai v2.x is no longer supported. Please upgrade to the latest 3.x version.

github-actions[bot] commented 1 year ago

This issue is stale, because it has been open for 45 days with no activity. Remove the stale label or comment, or this will be closed in 7 days.

github-actions[bot] commented 1 year ago

This issue was closed, because it has been stalled for 7 days with no activity.