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

Jump to bottom

Automatic task cleanup (without resumability) #4713

Closed
wants to merge 41 commits into from
Closed

Automatic task cleanup (without resumability) #4713

wants to merge 41 commits into from

Conversation

bentsherman
Copy link
Member

Spun off from #3849 without the WIP resumability piece.

@bentsherman
Add initial task graph and metadata json file
47d0168 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add task inputs and outputs to conrete DAG
ae67027 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Fix failing tests
8f95cd6 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Use path-based APIs to get file metadata
9f11e4b 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Merge branch 'master' into ben-task-graph
db6aed1 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Use buffer to compute checksum
8456892 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add support for temporary output paths
77f2cdc 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Fix failing test
3e55ad5 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add caveat about overlapping output channels [ci skip]
e307f75 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Delete files instead of emptying them (now supports directories and r…
08881b0 
…emote paths)

Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add `eager' cleanup option
0cf07ec 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Fix concurrency issues [ci fast]
73b2f3b 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Merge branch 'master' into ben-task-graph-pull
0dd98d6
@bentsherman
Merge branch 'master' into ben-task-graph-pull
0f505d3 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Replace synchronized with lock
e81e584 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Merge branch 'ben-task-graph-pull' into 452-eager-cleanup
ba2e7a6 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Remove dependency on task graph branch
f7bcfa8 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Use downstream tasks to determine lifetime for task cleanup
4d90e27 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Rename TemporaryFileObserver to TaskCleanupObserver
dd23b2a 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Merge branch 'master' into 452-eager-cleanup
6a34be6 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Wait for output files to be published
ff08984 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Log warning if eager cleanup is used with incompatible publish modes
9b343b6 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add eager cleanup for individual output files
6b5a820 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add warning about includeInputs with eager cleanup
c42f249 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add resumability
8b66c39 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Upgrade kryo to 5.4.0 (#3562)
43c939d 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add serializer for HashCode
5a78cf9 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Fix failing tests
7b02b8e 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Revert Kryo upgrade
c727c05 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add kryo instantiator for HashCode
d37ee1f 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Remove HashCode serializer
859e96b 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add log messages, fix infinite hang
c89811c 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add support for includeInputs
a034f60 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Update log messages
9741b1c 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add lazy, eager, aggressive cleanup strategies
3aae1af 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Merge branch 'master' into 452-eager-cleanup
08342dd 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Add thread pool for task cleanup
3498c5d 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Minor improvement to checkCachedOutput()
c421617 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Merge branch 'master' into 452-eager-cleanup
5b05aad 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
minor edits
9ce10dc 
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Remove WIP resumability
b73c6bb 
Signed-off-by: Ben Sherman <[email protected]>
@netlify Netlify
Copy link

netlify bot commented Feb 5, 2024 •
edited
Loading

✅ Deploy Preview for nextflow-docs-staging ready!

Name Link
🔨 Latest commit b73c6bb
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/65c1058f7d39420008439117
😎 Deploy Preview https://deploy-preview-4713--nextflow-docs-staging.netlify.app/config
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@bentsherman bentsherman marked this pull request as ready for review February 5, 2024 15:58
@bentsherman bentsherman mentioned this pull request Feb 5, 2024
Open
@CarsonJM CarsonJM mentioned this pull request Feb 16, 2024
Merged
11 tasks
@AlexVCaron AlexVCaron mentioned this pull request Feb 23, 2024
Open
robsyme
robsyme reviewed Mar 1, 2024
View reviewed changes
modules/nextflow/src/main/groovy/nextflow/trace/TaskCleanupObserver.groovy
// delete task
threadPool.submit({
try {
FileHelper.deletePath(task.workDir)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like we remove the whole task working directory, which would also destroy the .command files. These files are used by the Seqera Platform and are generally useful for task provenance. Would it be sensible to consider whitelisting the Nextflow (and maybe Fusion) dotfiles from deletion?

Copy link
Member Author

Choose a reason for hiding this comment

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

My plan was to save this provenance information in the .nextflow metadata cache, but right now that code is sitting in #3849. This information can also be preserved by using nf-prov

How else are they used by Seqera Platform?

Copy link
Member Author

Choose a reason for hiding this comment

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

@robsyme I managed to move this PR to a plugin, so feel free to make an issue there if you want to continue this discussion about deleting the task directory.

I could add a config option to control whether to delete the task directory or only the outputs. Also, the upcoming workflow output DSL might make it very difficult to know when a task directory could be deleted, so it might become a moot point

@bentsherman
Copy link
Member Author

Closing in favor of nf-boost plugin

@bentsherman bentsherman deleted the 452-eager-cleanup-m1 branch March 24, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robsyme robsyme robsyme left review comments

@pditommaso pditommaso Awaiting requested review from pditommaso

Assignees

@bentsherman bentsherman

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bentsherman @robsyme