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

cleanup performed on symlinks in work dir #105

Closed
christophmluscher opened this issue Sep 16, 2022 · 8 comments · Fixed by #154
Closed

cleanup performed on symlinks in work dir #105

christophmluscher opened this issue Sep 16, 2022 · 8 comments · Fixed by #154

Comments

@christophmluscher
Copy link
Contributor

Hey,

I get this error while running the sisyphus manager.

[2022-09-16 18:44:21,738] INFO: clean up: work/i6_core/datasets/librispeech/DownloadLibriSpeechCorpusJob.8iqSB1tz3OMD
tar (child): finished.tar.gz: Cannot open: Permission denied
tar (child): Error is not recoverable: exiting now 
[2022-09-16 18:44:21,874] WARNING: Could not clean up work/i6_core/datasets/librispeech/DownloadLibriSpeechCorpusJob.8iqSB1tz3OMD: Command '['tar', '-czf', 'finished.tar.gz', 'job.save', 'input', 'finished', 'last_user', 'log.run.1', 'engine', 'finished.run.1', 'usage.run.1', 'submit_log.run']' died with <Signals.SIGPIPE: 13>.
[2022-09-16 18:44:21,875] WARNING: Could not clean up work/i6_core/tools/download/DownloadJob.0UXAqd5DuQG7: Command '['tar', '-czf', 'finished.tar.gz', 'last_user', 'submit_log.run', 'finished.run.1', 'usage.run.1', 'log.run.1', 'input', 'job.save', 'engine', 'finished']' died with <Signals.SIGPIPE: 13>.

The sisyphus job dir is a symlink to location I have no write access.
I think there is a wrong assumption here. That you actually do not what to clean up a symlinked sisyphus job since you cannot reliably say that it belongs under your control

@christophmluscher
Copy link
Contributor Author

@critias what is your opinion?

@Atticus1806
Copy link
Contributor

Atticus1806 commented Sep 16, 2022

There was a fix that this at least only happens once #66

@christophmluscher
Copy link
Contributor Author

Thanks for the pointer. I already had that commit applied.

But what I mean is a more general question: why does a sisyphus manager have control over a sisyphus job dir which is not located in the corresponding work dir?!
I think this is not how it should be handled. But maybe I am missing something or design choice I am not aware of :)

@critias
Copy link
Contributor

critias commented Sep 17, 2022

Well, I wouldn't say it was a design choice. I just never checked this and so far nobody wanted it changed 😄

I frequently setup an experiment directory at one place, created the work directory somewhere else and created a symlink to it. So only cleaning directories in the corresponding work dir would skip all jobs in that case. Checking if the job directory itself is not a symlink (e.g. work/i6_core/datasets/librispeech/DownloadLibriSpeechCorpusJob.8iqSB1tz3OMD should not be a symlink) and that it belongs to the same user sounds like a good heuristic.

@christophmluscher
Copy link
Contributor Author

We are creating common baselines in a shared work dir from which everybody can import. To reduce data duplication, we want to use symlinks. But this sounds like a good solution to me. I can make the PR, if you don't have time.

@critias
Copy link
Contributor

critias commented Sep 20, 2022

Thanks, a PR would be great.

@Marvin84
Copy link

Hi @christophmluscher, any update on this?

@albertz
Copy link
Member

albertz commented Nov 20, 2023

Checking if the job directory itself is not a symlink (e.g. work/i6_core/datasets/librispeech/DownloadLibriSpeechCorpusJob.8iqSB1tz3OMD should not be a symlink) and that it belongs to the same user sounds like a good heuristic.

Is the second check (belongs to the same user) really good or necessary? I think just the first check (never cleanup symlinked job dirs) is more robust. Maybe I created the shared work dir and then in another separate Sisyphus setup, I would also not want that it touches my own externally symlinked job dirs.

If you agree, I probably would implement this soon. Should this be an option or just the new behavior? If this is supposed to be an option, what should be its default?

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

Successfully merging a pull request may close this issue.

5 participants