Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Server: caching improvements #152

Merged
merged 10 commits into from Nov 3, 2014
Merged

Server: caching improvements #152

merged 10 commits into from Nov 3, 2014

Conversation

mwmiller
Copy link
Contributor

  • Cache perl module status (DDG/App::DuckPAN) for 4 hours, by default
  • Similarly cache all downloaded assets (including the HTML pages)
  • Add option --cachesec/-c to control cache lifetime. -c=0 is very similar to -f
  • Reorganize asset-holding attributes to make it easier to understand how and where to add them
  • Use a standardized way to pull from cache and concatenate (if necessary), including type-specific comments on whence they come
  • Path::Tiny objects pushed "closer to the top" for easier reasoning about the files
  • Error out (and delete cache) when file is not fully received
  • Add download progress bar for more human-friendly output.
  • Minor improvement to, and expanded use of, exit_with_msg

@moollaza
Copy link
Member

@mwmiller nicely done. What exactly was the problem? And how does this fix it? I don't fully understand what's happening differently looking at the diff

@mwmiller
Copy link
Contributor Author

What exactly was the problem?

The short version is that ddh6 somehow has different stuff on its SERP than duckduckgo.com. This means we can't find a particular asset (notably, in this case, what becomes the page_template_files). This leads to a variety of problems.

This fix, then, just assumes we're OK with empty files in cases where they aren't included on the SERP. I'd like to put a little time into fixing this in a more general way.

@moollaza
Copy link
Member

Ah. @mwmiller I see what the problem is. DDH6 is in dev mode. With the recent Grunt changes, the names of the dev versions of the required JS have changes. Base.js and Serp.js are the new names. Those need to be updated in the regex that check for page assets and I think the problem should be resolved if I'm correct.

@moollaza
Copy link
Member

i.e. https://github.com/duckduckgo/p5-app-duckpan/blob/mwm/missing_files/lib/App/DuckPAN/Cmd/Server.pm#L265 should look for `/base.js/ is stead of "duckduck"

@mwmiller
Copy link
Contributor Author

@moollaza I figured this was (more or less) what was happening, but I still think that the regex-based matching is pretty fragile.

@moollaza
Copy link
Member

@mwmiller that last commit is great. Looks like you need to rebase though?

@mwmiller
Copy link
Contributor Author

@moollaza I meant to tag this with WIP, too. I got distracted and haven't completed my testing.

@moollaza
Copy link
Member

@mwmiller ah, no prob 👍

@mwmiller mwmiller force-pushed the mwm/missing_files branch 2 times, most recently from 1a2014e to 4429a05 Compare November 1, 2014 06:41
@mwmiller mwmiller changed the title Server: deal better with changing page structure. Server: caching improvements Nov 1, 2014
@mwmiller mwmiller mentioned this pull request Nov 1, 2014
Closed
@mwmiller
Copy link
Contributor Author

mwmiller commented Nov 1, 2014

I am removing the WIP label from this, because I feel it is done. It could still use copious testing more environments. Assuming it works as well for everyone else as it does for me, it's a huge win for getting a working environment relatively quickly on a poor-quality internet connection.

In a few moments, I'll update the description to more completely indicate what is included.

@mwmiller
Server: deal better with changing page structure.
21da700 
This is still far too tied to a particular page layout, but it won't die
now.

Fixes #151.  (With improvements to come)
@mwmiller
Server: clean up file cache loading somewhat.
88968e7 
This makes a more straight-forward path through the code to understand
what is happening.

Also more commenting on the files themselves, to make it easier to
review in-browser.
@mwmiller
Server: further refactor cachefile handling.
cfefaab 
This is still not _exactly_ what I want, but we're getting closer.
@mwmiller
Server: further streamline caching structure.
cb803fa 
This allows for a much better experience when working on a slow link.

- Allow caching of everything (including perl module status and HTML
  pages) for 4 hours, by default.
- Provide an option to change the cache lifetime (-c=0 is more or less
  equivalent to -f)
- Store un-changed files in cache, updating them just before preparing
  to serve.
- Always read from cached file, which helps ensure they will be
  reusable, even when the force is on.

Still a problem: country.json
@mwmiller
Server: still better caching information.
e0a67f2 
- Make it possible to recognize paritally downloaded files, exiting
  and removing them from the cache.
- Add Term::ProgressBar for download tracking.
- Replace die with exit_with_msg.
- Update exit_with_msg for better visual on multi-line messages.
@mwmiller
Server: do not trust things external things in the dist-cache
bc607de 
Only copy over files without external soruces (notably duckpan.js, at
present)... everything else is either gotten fresh or cache-checked.

I don't even know how these managed to get distributed to me, but ah,
well.
@mwmiller
Server: clean up output a bit.
7b3adca 
Also:

- Remove now incorrect comment.
- Simplify the external/dist logic a bit.
@mwmiller
Config: clean up path separation.
4d47a01 
This straightens out the directory structure, while also expanding the
application of path().
@mwmiller
Copy link
Contributor Author

mwmiller commented Nov 1, 2014

@moollaza Getting to your actual point, now that I've gone off the deep-end and rewritten much of the caching. 😁

Do I want to add 'base' to the 'duckduck' and 'serp' to the 'duckpan' regexes?

@moollaza moollaza self-assigned this Nov 1, 2014
@moollaza
Copy link
Member

moollaza commented Nov 2, 2014

@mwmiller yes the dev files we need are base.js, duckpan.js and serp.js which are the same as d####.js, dpan####.js and g####.js respectively. We can remove duckduck.js, it no longer exists under that name.

mwmiller and others added 2 commits November 2, 2014 10:48
@mwmiller
Server: store assets on a per-host basis.
8dbc136 
If you're changing the hostname, you're probably looking for
different assets.

Also, look for new gruny-style filenames which addresses #151.
@moollaza
Remove old filenames from regex, switch around base and duckpan
64baf73
@moollaza
Copy link
Member

moollaza commented Nov 3, 2014

@mwmiller made a small update to your latest commit -- this should be good now 👍

@moollaza
Copy link
Member

moollaza commented Nov 3, 2014

Tested and looks good. @mwmiller thanks a lot for the tremendous improvements 😃

moollaza added a commit that referenced this pull request Nov 3, 2014
@moollaza
Merge pull request #152 from duckduckgo/mwm/missing_files
428d5a3 
Server: caching improvements
@moollaza moollaza merged commit 428d5a3 into master Nov 3, 2014
@moollaza moollaza deleted the mwm/missing_files branch November 3, 2014 18:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@moollaza moollaza

Labels
Improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mwmiller @moollaza