Cache downloads globally #2185

Olympic1 · 2017-11-14T01:13:33Z

This is the beginning of a global cache for downloads. There is a test for read/write access, and an option for moving a cache. Currently, there is no way to go from the local cache in the client into the global cache.

Closes #584, closes #960

HebaruSan

Nice! It's exciting to see this one moving forward; it's one of those "why doesn't it already work this way" projects, but of course someone has to actually write the code to make it happen.

I previously started my own version of this change in a local branch, so some of my comments are based on that investigation. I had it to the point of automatically importing all the old separate caches into the global cache at launch, mostly by rewriting KSP.DownloadCacheDir(). I can share that branch if needed, but it's not too different from what you already have.

HebaruSan · 2017-11-14T01:39:26Z

Core/Net/NetFileCache.cs

+                // If the above didn't work, fall back to "~/.local/"
+                if (string.IsNullOrWhiteSpace(baseDirectory))
+                {
+                    baseDirectory = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Personal), ".local/share");


Can we fully allow Path.Combine to handle the path separators?

Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Personal), ".local", "share")

HebaruSan · 2017-11-14T01:44:12Z

Core/Net/NetFileCache.cs

+                // If the above didn't work, fall back to "~/.local/"
+                if (string.IsNullOrWhiteSpace(baseDirectory))
+                {
+                    baseDirectory = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Personal), ".local/share");


I think we should let Mono handle the specifics of the path to use; it gave me a ~/.local/share prefix for Environment.SpecialFolder.LocalApplicationData on Linux, and a similarly good location on WIndows; can we use that?

// /home/MyUserName/.local/share/CKAN/downloads // C:/Users/MyUserName/AppData/Local/CKAN/downloads Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "CKAN", "downloads")

HebaruSan · 2017-11-14T01:46:57Z

Core/Net/NetFileCache.cs

+            // TODO: Start a new transaction
+
+            // Get a list of all the files in the old cache and move them over
+            var files = Directory.GetFiles(cachePath);


I believe Directory.EnumerateFiles(cachePath, "*") would be more performant for very large caches, as it does not load the entire list of files into memory all at once but instead loads them one at a time for our code to handle.

HebaruSan · 2017-11-14T01:48:22Z

Core/Net/NetFileCache.cs

+        /// Moves the old cache folder to a new one.
+        /// </summary>
+        /// <param name="newPath">The path to move to.</param>
+        /// <param name="move"></param>


Description missing

HebaruSan · 2017-11-14T01:52:19Z

Core/Net/NetFileCache.cs

+
+                if (move)
+                {
+                    tx_file.Move(file, newFilePath);


At the moment my CKAN cache is on a non-HOME partition and slightly larger than the free space on my HOME partition. This loop could fill my disk; we at least need exception handling, if not explicit space checks.

$ du -h /media/DataTrove/Games/CKAN_downloads ; df -h $HOME 28G /media/DataTrove/Games/CKAN_downloads Filesystem Size Used Avail Use% Mounted on /dev/sda1 217G 180G 27G 88% /

Will do some research for checking disk space later this week.

HebaruSan · 2017-11-14T01:54:32Z

Core/Net/NetFileCache.cs

+            foreach (string file in files)
+            {
+                if (tx_file.FileExists(file))
+                    tx_file.Delete(file);


This loop looks like it should not be here. If move was true, then we moved the files and they're no longer there to delete. If move was false, then we copied them and presumably want to retain the old copies.

HebaruSan · 2017-11-14T01:56:34Z

Core/Net/NetFileCache.cs

+            }
+
+            // Append the CKAN folder structure
+            string directory = Path.Combine(baseDirectory, "CKAN/downloads");


Same comment about letting Path.Combine handle all path separators.

Path.Combine(baseDirectory, "CKAN", "downloads")

HebaruSan · 2017-11-14T02:02:27Z

Core/Net/NetFileCache.cs


-        public NetFileCache(string _cachePath)
+        public NetFileCache(string _cachePath = null)


This parameter is still being passed from KSP.cs:61, so this change would not yet have an effect in the full app. The KSP.DownloadCacheDir() function probably needs to be retired, with some kind of migration process for old downloads.

Rewrote the KSP.DownloadCacheDir() and cache gets moved when new path was specified.

HebaruSan · 2017-11-14T02:08:33Z

Core/Net/NetFileCache.cs

+            // Check that we have list permission
+            try
+            {
+                Directory.GetFiles(newPath);


This will load the entire list of files in the target directory into memory, which could be a performance hit if there are already a bunch of files there (e.g., if merging multiple separate instance caches). Is there another check we could do instead? What if we just created the test file?

HebaruSan · 2017-11-14T02:10:18Z

Core/Net/NetFileCache.cs

+
+            foreach (string file in files)
+            {
+                string newFilePath = Path.Combine(newPath, Path.GetFileName(file));


If we're combining multiple old caches, there's a good chance there will be collisions, as eliminating redundant downloads and disk space is one of the reasons this project was requested. We should check whether newFilePath already exists and do something intelligent if it does. Maybe print an error, maybe just delete it if we're moving and the file contents are the same.

politas · 2017-11-14T09:06:01Z

I made a start on this one, too! I have a CLI Cache command that can probably be converted to work with this. I've opened a PR on your branch with my CLI code. If you want to look at the rest of what I had done (for whatever it is worth), it is here.

Olympic1 · 2017-11-20T00:40:34Z

Core functions are in place this will mostly be bugfixes now.

@politas, I wil add your CLI code later. Same for Test cases if needed.

Genjokoan · 2017-12-01T03:16:19Z

I am new to KSP and looked for how to do this, which led me here. I can do testing for you on Mac OS and Windows 10. My Mac kung fu is okay, windows not so much though it is my main KSP machine.

Olympic1 · 2017-12-01T06:16:19Z

Thanks that you want to test this out, this needs some testing on Mac OS. Here is a compiled version of ckan.exe and netkan.exe. Let me know if you come across any issues.

CKAN-GlobalCache.zip

Genjokoan · 2017-12-01T20:23:45Z

Issues, yes. I have a Mac Mini (Late 2012) but play on a PC. I installed your ckan.exe and getting it to run involved a little trial and a lot of error. Even with the wiki install-on-a-mac I think someone with purely point-and-click mindset would give up before getting the app running.
Now, running. I have my monitor set for 1440p and the difference between the Mac and PC version is remarkable. The Mac version's graphics were inconsistent. I can provide screen shots if you want them.
In editing the settings, I would set the cache directory and it would not take. Set it again, no joy. Quit, restart, set, no joy. I kept at it and after about three restarts, the setting stuck, though when you have the setting box open, the cache directory appears blank. This trend continued when selecting an install to modify, with the cache first going to the install directory and only writing to the "cache directory" after several restarts.
Changing to a different ksp install required a restart or the results were unpredictable. After the first round of selecting mods to apply, going back for more mods - after clicking on one the app would go to the apply now screen with no way of going back to select more.
I am happy to keep testing. If I were looking at this as a regular player, I would not use it. I have not tried this version on my PC yet. I will do so today. The "regular" version works okay fine on my PC.
Sorry if this is too chatty. I have not participated in this community process for decades. If you want to asses my background before rating any feedback, email me outside of git.
It has been a while but if you need screenshots I can provide them (here or offline?) and just maybe I can record some screen video.

Genjokoan · 2017-12-02T02:39:31Z

On the plus side, it works great on windows 10 pro. IF you start with clean installs and double check the settings before you start accessing the cache. I started with a test setup that had three installs in various states of mods applied and the results were unpredictable. The first install I tested worked okay fine. Switching to other installs, I could not get them to transition to the one, new cache. My three new installs are working well after adding/removing mods on each many times. I will put more effort into testing on a Mac later.

Olympic1 · 2018-03-21T21:36:54Z

wtf, how did this happen? I didn't push anything to this branch or repo. Now I lost everything

HebaruSan · 2018-03-21T21:59:16Z

Apologies for my previous message, I think that was a copy of my own changes on this topic. Here's what I have of yours:

https://github.com/HebaruSan/CKAN/tree/olympic1-shared-cache

techman83 · 2018-03-22T02:03:19Z

If you know the commit IDs, you should be able to cherry pick them from the refs in your local repo. Checkout a new branch from master and cherry pick them all. It's hard to lose stuff, but it's also hard to find stuff sometimes :-/

Genjokoan · 2018-03-22T02:21:08Z

All I have is the exe's you sent me.

politas · 2018-03-22T09:12:53Z

This is what I have - I've reverted the commit of mine you didn't accept and cherry-picked all the commits I had in my local view of your branch https://github.com/politas/CKAN/tree/feature/cache-global

That has changes up to Nov 25

Olympic1 · 2018-03-24T23:11:52Z

Ok, got it back to where I had it and triggered it so it to reopen.

I don't know why it included the last merged PR but it should be fine.

CKAN-GlobalCache.zip

HebaruSan · 2018-08-27T00:19:44Z

The problem with the permissions test is that C# doesn't have octal numeric literals, just hex and decimal. So 0000400 is 400 decimal, etc., and the final chmod call doesn't re-enable all owner permissions.

EDIT: That also explains why the latest commit passed, btw. 100 | 200 | 400 = 492, not 700, so that substitution changed the value being passed to chmod.

HebaruSan · 2018-08-27T01:13:19Z

Core/KSP.cs

-            return KSPPathUtils.NormalizePath(
-                Path.Combine(CkanDir(), "downloads")
-            );
+            var registry = RegistryManager.Instance(this).registry;


This causes all of the instances of the registry to be loaded at start. Since a current registry is 20 MB, this would be quite a delay on systems with many instances. It also causes an unworkaroundable exception if you have an instance with a corrupted registry.json (as I do).

HebaruSan · 2018-08-27T01:14:25Z

Tests/Core/Cache.cs

+
+        private const uint S_IRUSR = 0000400;
+        private const uint S_IWUSR = 0000200;
+        private const uint S_IXUSR = 0000100;


Try Convert.ToUInt32("0400", 8), etc. C# doesn't have octal numeric literals.

HebaruSan · 2018-08-30T00:38:15Z

Core/Net/NetFileCache.cs

+                // If the above didn't work, fall back to "~/.local/"
+                if (string.IsNullOrWhiteSpace(baseDirectory))
+                {
+                    baseDirectory = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Personal), ".local", "share");


This is the same as SpecialFolder.LocalApplicationData on Linux as far as I can tell. Opportunity to consolidate with below code here.

HebaruSan · 2018-08-31T18:23:40Z

Cmdline/Action/Cache.cs

+
+namespace CKAN.CmdLine
+{
+    public class Cache : ISubCommand


A new VerbOption should be added to the Actions class in Options.cs to make this new command show up in ckan help.

Olympic1 added Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality In progress We're still working on this Pull request ★★☆ labels Nov 14, 2017

Olympic1 added this to the 1.24.0 milestone Nov 14, 2017

Olympic1 self-assigned this Nov 14, 2017

Olympic1 added In progress We're still working on this ★★☆ and removed In progress We're still working on this ★★☆ labels Nov 14, 2017

HebaruSan requested changes Nov 14, 2017

View reviewed changes

Olympic1 force-pushed the feature/cache-global branch from b45b438 to 2339ae9 Compare November 19, 2017 11:56

Olympic1 mentioned this pull request Nov 20, 2017

Cache downloads globally KSP-CKAN/CKAN-core#135

Closed

Olympic1 modified the milestones: 1.24.0, 1.26 Mar 18, 2018

More verbose and accurate version displays

4b510d3

Olympic1 closed this Mar 21, 2018

Olympic1 force-pushed the feature/cache-global branch from ad78bf3 to 5479760 Compare March 21, 2018 19:43

Olympic1 removed Enhancement New features or functionality Pull request ★★☆ labels Mar 21, 2018

Merge KSP-CKAN#2382 More verbose and accurate version displays

1835b2d

Cache downloads globally

4d26bdb

Olympic1 reopened this Mar 24, 2018

Try these permissions

7741cdb

HebaruSan mentioned this pull request May 6, 2018

[Enhancement suggestion] Purge old downloads from cache #2438

Closed

HebaruSan reviewed Aug 27, 2018

View reviewed changes

HebaruSan reviewed Aug 30, 2018

View reviewed changes

HebaruSan reviewed Aug 31, 2018

View reviewed changes

HebaruSan mentioned this pull request Oct 9, 2018

One Cache to Rule Them All #2535

Merged

politas closed this in #2535 Oct 13, 2018

Olympic1 deleted the feature/cache-global branch October 13, 2018 18:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cache downloads globally #2185

Cache downloads globally #2185

Olympic1 commented Nov 14, 2017 •

edited

Loading

HebaruSan left a comment

HebaruSan Nov 14, 2017

HebaruSan Nov 14, 2017

HebaruSan Nov 14, 2017

HebaruSan Nov 14, 2017

HebaruSan Nov 14, 2017

Olympic1 Nov 20, 2017

HebaruSan Nov 14, 2017

HebaruSan Nov 14, 2017

HebaruSan Nov 14, 2017

Olympic1 Nov 20, 2017

HebaruSan Nov 14, 2017

HebaruSan Nov 14, 2017

politas commented Nov 14, 2017 •

edited

Loading

Olympic1 commented Nov 20, 2017 •

edited

Loading

Genjokoan commented Dec 1, 2017

Olympic1 commented Dec 1, 2017 •

edited

Loading

Genjokoan commented Dec 1, 2017

Genjokoan commented Dec 2, 2017

Olympic1 commented Mar 21, 2018

HebaruSan commented Mar 21, 2018

techman83 commented Mar 22, 2018

Genjokoan commented Mar 22, 2018

politas commented Mar 22, 2018 •

edited

Loading

Olympic1 commented Mar 24, 2018 •

edited

Loading

HebaruSan commented Aug 27, 2018 •

edited

Loading

HebaruSan Aug 27, 2018

HebaruSan Aug 27, 2018

HebaruSan Aug 30, 2018

HebaruSan Aug 31, 2018


		public NetFileCache(string _cachePath)
		public NetFileCache(string _cachePath = null)

Cache downloads globally #2185

Cache downloads globally #2185

Conversation

Olympic1 commented Nov 14, 2017 • edited Loading

HebaruSan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

politas commented Nov 14, 2017 • edited Loading

Olympic1 commented Nov 20, 2017 • edited Loading

Genjokoan commented Dec 1, 2017

Olympic1 commented Dec 1, 2017 • edited Loading

Genjokoan commented Dec 1, 2017

Genjokoan commented Dec 2, 2017

Olympic1 commented Mar 21, 2018

HebaruSan commented Mar 21, 2018

techman83 commented Mar 22, 2018

Genjokoan commented Mar 22, 2018

politas commented Mar 22, 2018 • edited Loading

Olympic1 commented Mar 24, 2018 • edited Loading

HebaruSan commented Aug 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Olympic1 commented Nov 14, 2017 •

edited

Loading

politas commented Nov 14, 2017 •

edited

Loading

Olympic1 commented Nov 20, 2017 •

edited

Loading

Olympic1 commented Dec 1, 2017 •

edited

Loading

politas commented Mar 22, 2018 •

edited

Loading

Olympic1 commented Mar 24, 2018 •

edited

Loading

HebaruSan commented Aug 27, 2018 •

edited

Loading