<div dir="ltr"><div dir="ltr"><div dir="ltr"><div>I was able to get this working, and with a double ack I merged it. Further refinements are welcome.</div><div><br></div><div><a href="https://github.com/pulp/pulpcore/pull/486">https://github.com/pulp/pulpcore/pull/486</a></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 10, 2020 at 9:56 AM Brian Bouterse <<a href="mailto:bmbouter@redhat.com">bmbouter@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Here's a summary of what I've learned about our options to resolve the two issues linked in my original email. tl;dr we should maintain a "move" behavior like we have now whenever that file isn't file:///.</div><div><br></div><div>It's inefficient to duplicate data that was uploaded from the user, or downloaded with the http downloader (this is a speed issue). It's also undesirable in size; if you're syncing a large RHEL you would spend maybe 20ish gigs in the working directory, and another 20 in the storage backend. This is undesirable, so we should stick with moving data whenever we can.<br></div><div><br></div><div>So for the case of syncing from file:///, that is the one where we need to copy data. So how should we efficiently copy data?</div><div><br></div><div>reflinks would be sweet for copying and I prototyped it, but Pulp would pick up a dependency on 'reflink' <a href="https://reflink.readthedocs.io/en/latest/" target="_blank">https://reflink.readthedocs.io/en/latest/</a> which requires gcc client build at install time. That causes it to fail on Travis. I don't think reflink is a significant enough benefit to cause our pip installs of pulpcore to require a non-pip dependency. What do you think about this tradeoff?<br></div><div><br></div><div>Also, reflinks will matter less over time. We would use shutil.copyfile() to perform the copying and in Python 3.8 they already improved the implementation when the source and destination are able to share inodes ( <a href="https://bugs.python.org/issue33671" target="_blank">https://bugs.python.org/issue33671</a> ). Also they are adding new calls with a full reflink implementation in Python 3.9 AIUI ( <a href="https://bugs.python.org/issue37157" target="_blank">https://bugs.python.org/issue37157</a> ).</div><div><br></div><div>Feedback and ideas are welcome. On the current course I'm going to focus on having file:/// copy, leaving the existing "move" as is. Also I'm going to update the Storage backend to use the _save() method instead of save() which is the recommended way django says to customize our storage system.</div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 8, 2020 at 6:10 PM Brian Bouterse <<a href="mailto:bmbouter@redhat.com" target="_blank">bmbouter@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>It's not finished, but I have a POC with tests passing locally for me here: <a href="https://github.com/pulp/pulpcore/pull/486" target="_blank">https://github.com/pulp/pulpcore/pull/486</a> It does use the reflink library and falls back to copy.<br></div><div><br></div><div>What still needs to be done is:</div><div>* To ensure files are being cleaned up in the case of uploads.</div><div>* Also I think we have a bug where some cases may not cleanup the worker directory which would now leave those files around forever.</div><div><br></div><div>I will resume on those things tomorrow.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 8, 2020 at 12:32 PM Brian Bouterse <<a href="mailto:bmbouter@redhat.com" target="_blank">bmbouter@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 8, 2020 at 12:02 PM Simon Baatz <<a href="mailto:gmbnomis@gmail.com" target="_blank">gmbnomis@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, Jan 07, 2020 at 04:45:54PM -0500, Brian Bouterse wrote:<br> > We had two bugs filed recently [0][1] which suggest that when using the<br> > default backend for Pulp, i.e. pulpcore.app.models.storage.FileSystem<br> > Pulp should not be "moving" files. This is the default behavior Django<br> > gives us, and it destroys data when sync'ed from file:/// for example<br> > [1].<br> <br> I am surprised that file:/// should be supported at all. This means<br> that a worker process must be able to access basically any file on the<br> system. Is this covered by the (upcoming?) SELinux policy? I would<br> expect workers to be more constrained than that. Or do we expect<br> the user to label files before importing?<br></blockquote><div>The user would need to label the files ahead of time. The use case is that a large amount of content is stored on a hard drive which is mounted on the worker. A lot of setups of Pulp use this as a mass-import method before switching their sync's back to the CDN.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> > I propose that with 3.1 we fix this bug by switching<br> > pulpcore.app.models.storage.FileSystem to leave files in place and<br> > either hard-link (same filesystem) or copy (different filesystem).<br> <br> We should not use hard links:<br> <br> - On systems with sysctl "fs.protected_hardlinks" enabled (Ubuntu, Fedora),<br> the files would have to be owned by the pulp user to be able to<br> create hard-links at all.<br> - On systems with SELinux enabled, these hard links will share<br> the labels.<br> - Imported hard links are not protected against modification of the<br> original file.<br> <br> We could try to reflink the file and fall-back to copy (like "cp<br> --reflink=auto" does)<br></blockquote><div>Great idea; let me try doing it this way. I'll link to my POC pr when it's up. Feel free also to share one. <br></div></div></div> </blockquote></div> </blockquote></div> </blockquote></div>