Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

add workflow cleaner to clean expired workflows periodically #1609

Closed
wants to merge 3 commits into from
Closed

add workflow cleaner to clean expired workflows periodically #1609

wants to merge 3 commits into from

Conversation

kensou97
Copy link

Hi, I ran into a problem while using redis persistence, which has already been discussed in some issues like #1315. Redis memory usage kept growing and random data including some metadata got evicted.
So I add a workflow cleaner in RedisExecutionDAO. It records the timestamp and workflowId in zset (timestamp as score, workflowId as value) when a workflow is created. A background thread will find and clean the expired workflows (indicated by config workflow.cleaner.expire.seconds) periodically. The period is indicated by config workflow.cleaner.period.seconds. If there are too many expired workflows, cleaner will process them batch by batch. The batch size is indicated by config workflow.cleaner.batch.size.
The cleaner is not meant to clean data in IndexDAO or other db implementation, because the disk-based storage may not have this problem. It works well in my environment and I hope this would help someone else.

@kensou97 kensou97 closed this Mar 27, 2020
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3887

  • 15 of 35 (42.86%) changed or added relevant lines in 2 files are covered.
  • 16 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.2%) to 68.786%

Changes Missing Coverage Covered Lines Changed/Added Lines %
redis-persistence/src/main/java/com/netflix/conductor/dao/dynomite/RedisExecutionDAO.java 12 32 37.5%
Files with Coverage Reduction New Missed Lines %
cassandra-persistence/src/main/java/com/netflix/conductor/dao/cassandra/CassandraEventHandlerDAO.java 8 69.86%
cassandra-persistence/src/main/java/com/netflix/conductor/dao/cassandra/CassandraMetadataDAO.java 8 55.79%
Totals Coverage Status
Change from base Build 3882: -0.2%
Covered Lines: 11393
Relevant Lines: 16563

💛 - Coveralls

@codecov
Copy link

codecov bot commented Mar 27, 2020

Codecov Report

Merging #1609 into dev will decrease coverage by 0.14%.
The diff coverage is 42.85%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #1609      +/-   ##
============================================
- Coverage     64.08%   63.93%   -0.15%     
- Complexity     3424     3429       +5     
============================================
  Files           273      273              
  Lines         16217    16252      +35     
  Branches       1448     1451       +3     
============================================
- Hits          10392    10391       -1     
- Misses         5104     5140      +36     
  Partials        721      721
Impacted Files Coverage Δ Complexity Δ
...m/netflix/conductor/core/config/Configuration.java 71.42% <100%> (+3.42%) 15 <3> (+3) ⬆️
...flix/conductor/dao/dynomite/RedisExecutionDAO.java 72.25% <37.5%> (-3.76%) 61 <2> (+2)
...ductor/dao/cassandra/CassandraEventHandlerDAO.java 68.49% <0%> (-10.96%) 15% <0%> (ø)
.../conductor/dao/cassandra/CassandraMetadataDAO.java 54.73% <0%> (-4.22%) 24% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f1fc99...e81b3bf. Read the comment docs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants