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

Do not write to the cache anywhere in build_target() #236

Closed
wlandau opened this issue Feb 5, 2018 · 5 comments
Closed

Do not write to the cache anywhere in build_target() #236

wlandau opened this issue Feb 5, 2018 · 5 comments

Comments

@wlandau
Copy link
Member

wlandau commented Feb 5, 2018

drake_build() should just return a target's value, and it should not cache errors or progress on its own. This is especially important for jobs deployed to remote locations that have no access to the cache. Transferring data should be the responsibility of the future package.

@kendonB
Copy link
Contributor

kendonB commented Feb 5, 2018

I recall at one point the value was returned using future (as well as written to disk) and the process was horribly slow and memory hungry for the host. For SLURM, this (at least) doubles the I/O writing; once to bring the data back to the host process and once to write it to disk from the host process. This will be a lot of work for the host process for a large project which usually has limited resources.

I find when running my project using future_lapply, after the slurm jobs complete, the host R process's memory usage blows up (slowly) in htop. Even if this is isn't real memory usage, it's still problematic as I'm running the host process on the shared build node. Is this the behavior you expect? Is the host process bringing back all the data to the host before writing to disk?

Ref: #115

@wlandau
Copy link
Member Author

wlandau commented Feb 5, 2018

So then we should think about a user-side option to cache inside or outside the future. Either way, the internals need to be cleaned up to account for all this.

@kendonB
Copy link
Contributor

kendonB commented Feb 5, 2018

I wonder if there's a future environment variable that indicates whether the future has access to the main disk? That might be able to shield the user from yet another argument to make

@wlandau
Copy link
Member Author

wlandau commented Feb 5, 2018

There may be, but I think there's also more to it than that: certain storr drivers are not threadsafe, and connections are lost in remote jobs. For example, storr_dbi() only works locally with one job. I would prefer a special setting here because keeping a master list of safe caches seems brittle.

@wlandau wlandau changed the title Do not write to the cache anywhere in drake_build() Do not write to the cache anywhere in build_target() Feb 7, 2018
@wlandau
Copy link
Member Author

wlandau commented Feb 7, 2018

I think we're good here: in #237 and #227, all we need to do is pay attention to where we use build_and_store() versus just_build(). It should not be hard now.

@wlandau wlandau closed this as completed Feb 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants