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.

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

Publishing a symlink when using fusion copies link (mock symlink) rather than link target #4967

Closed
robsyme opened this issue May 2, 2024 · 13 comments · Fixed by #5004
Closed

Comments

@robsyme
Copy link
Collaborator

robsyme commented May 2, 2024

Bug report

Given a simple workflow that creates a file

workflow {
    Channel.value("Seqera")
    | MakeJson
    | PublishIdCard
}

process MakeJson {
    input: val(name)
    output: tuple val(name), path('*.json')
    script: 
    """
    echo '{"name":"$name"}' > id.${name}.json
    """
}

process PublishIdCard {
    publishDir 'results'

    input: tuple val(name), path("jsons/*")
    output: path("symlink.json")
    script: "ln -s jsons/id.${name}.json symlink.json"
}

... and configuration that sets fusion:

workDir = 's3://my-bucket-name/work'
wave.enabled = true
fusion.enabled = true
docker.enabled = true
process.scratch = false
process.container = 'ubuntu:latest'

Expected behavior and actual behavior

I would expect the contents of the file published at ./results/symlink.json to include the json document

{"name":"Seqera"}

...but instead it is a text file containing:

jsons/id.Seqera.json

Notes

The .fusion.symlinks document in the task working directory of the PublishIdCard task contains the text symlink.json, but Nextflow doesn't seem to follow the symlink when publishing.

@bentsherman
Copy link
Member

Not sure this is worth supporting. Normally we detect fusion symlinks by checking if the output file is also an input, which is cheap because the list of inputs is in memory. But detecting this would require checking for .fusion.symlnks for every task, including subdirectories. This is because, though this example is simple, there is no way to know in general if a task will link inputs to outputs. So now you're doing a bunch of extra S3 calls for every task, even if you never use this pattern.

Instead, I would recommend that you either copy the file or declare the input file as an output directly (e.g. output: path("jsons/id.${name}.json")). I understand this might be more verbose compared to e.g. a single output directory containing a bunch of different files. It might be more readable when we have static types #4553 and can output a record type representing a directory of diverse output files.

@pditommaso

This comment was marked as outdated.

@pditommaso
Copy link
Member

However, this is introducing an increasing inconsistency between Fusion and non-fusion usage.

Normally this is not a problem because the OS knows how to handle symlinks, which obviously is not the case for Fusion.

I'm starting to think it should be the role of Fusion to concretize symlinks when referenced in the publishDir.

Now that were are introducing the task meta-comment, it should be an easier task. Adding @jordeu for visibility

@bentsherman
Copy link
Member

should be the role of Fusion to concretize symlinks when referenced in the publishDir

I assume you mean the process outputs? Because we are decoupling the publishing from the task execution with the workflow publish

So if a process output turns out to be a symlink -- whether because it was also an input or it was linked by the task, or some other reason -- Fusion should upload a copy to S3. Then Nextflow doesn't need to check for forwarded inputs when publishing files

This is actually necessary with the workflow publish, because files published by the workflow don't know the task they came from, so they can't check if they are a forwarded input

@pditommaso
Copy link
Member

No, I mean only publish, because non-publish output files will be used by Fusion in downstream task and, I guess, Fusion is able to handle properly

@bentsherman
Copy link
Member

Well, if we use the workflow publish instead of publishDir then the task doesn't know which outputs will be published

@jordeu
Copy link
Collaborator

jordeu commented May 3, 2024

What is the "the workflow publish"?

@bentsherman
Copy link
Member

Workflow publish (i.e. output) definition: #4784

@bentsherman
Copy link
Member

I was going to summarize the hybrid solution we discussed on the call, but thinking through it now I realize that it won't work. The workflow output definition makes it difficult for a task to know whether an output file will be published.

I think the best thing we can do instead is (1) give Fusion the output file patterns and (2) Fusion should always upload a copy if a symlink is an output file. This will only happen when an output file is a symlink, which is relatively uncommon. We can also remove the Fusion symlink logic from Nextflow.

@pditommaso
Copy link
Member

I think the best thing we can do instead is (1) give Fusion the output file patterns and (2) Fusion should always upload a copy if a symlink is an output file. This will only happen when an output file is a symlink, which is relatively uncommon. We can also remove the Fusion symlink logic from Nextflow.

it makes sense to me

@robsyme
Copy link
Collaborator Author

robsyme commented May 8, 2024

Giving Fusion the output patterns is a huge win because it provides the opportunity for Fusion to prioritize uploading particular files and potentially abandon the upload of non-output files when the task is complete.

@jordeu
Copy link
Collaborator

jordeu commented May 8, 2024

Distinguishing between input, intermediate, and output files in the working directory opens the door to many optimizations.

But still, I'm not convinced that it is a good idea to "materialize" all the outputs as objects when they are symbolic links. Imagine, for example, that the output is a symbolic link to a folder with thousands of small files. Materializing all these thousands of files in an object store is a costly operation.

@bentsherman
Copy link
Member

Keep in mind that it only heeds to happen when the user creates such a link or forwards an input as an output. These cases are uncommon and can be easily avoided by writing the pipeline differently

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

Successfully merging a pull request may close this issue.

4 participants