Support maxage for uplinks

[samcday]

Adds support for the maxage config property of uplinks.

[rlidwka]

Thanks for creating this pull request.

It works, but this solution will overwrite package.json's on every request. It is a big deal in terms of i/o (especially because it's a multi-step process to avoid race conditions). Etags are fine since they rarely change.

So I think I'm going to rewrite it using in-memory cache (and add a few missing timers we planned to add a few months ago).

[samcday]

Good point. It should be possible to address your concern without something as dramatic as a rewrite though. I'll look into this when I get a chance.

[samcday]

Wait a second, why would this be overwriting package.json on every request? I added the condition to also write package.json only if the fetched time doesn't match what we currently have. The fetched property should only be updated once we hit the upstream again.

[rlidwka]

Wait a second, why would this be overwriting package.json on every request?

On every request to upstream. If this option is on by default (which is desirable), and has a small value (a few minutes), taking into account npm's behaviour to create 100 requests at a time...

On the other hand, if you set up huge maxage and expect it to survive restarts, your approach is correct.

/me thinks about benchmarks

[samcday]

Oh, right. I was so focussed on my use-case I forgot other ones exist. Oops xD

Uh so that seems like it should still be easy to fix. I just need to short-circuit the need_change conditional if max-age is zero (no cache)

... Right? :)

[rlidwka]

It adds about 10% in the worst case when the uplink is very fast (i.e. local couchdb), 15 vs 17 seconds for 1000 sequential requests in my weird test case. Doesn't matter with default registry though.

I just need to short-circuit the need_change conditional if max-age is zero (no cache)

It's not about zero maxage, it's about very small maxage. If npm makes the same request twice in a row, we should probably respond with the same data by default. But if user does npm publish in the meantime (a few minutes maybe), we should not do so.

What is your use-case anyway?

[samcday]

I will be deploying sinopia to proxy our organisations internal NPM mirror. We have offices and build agents in a few different continents, I want to make sure "hot" packages are cached geographically close to each of these locations. Because we're also maintaining a complete mirror of the upstream NPM registry (to shield us from the seemingly endless outages it's been having lately), I can inexpensively follow registry package changes and invalidate caches at each proxy location, so I plan on setting a very high max-age (24 hours or more) to make sure NPM is blazingly fast for us.

[rlidwka]

Well, in this case you're right, it's the only logical option.

Thanks, I'll merge this.

Suggestions about how to not end up with two different pieces of code for the same function (depending on whether maxage is small or large) are appreciated. :)