Skip to content

Remove in-memory caches#36

Closed
dobesv wants to merge 6 commits into
cloudhead:masterfrom
dobesv:nocache
Closed

Remove in-memory caches#36
dobesv wants to merge 6 commits into
cloudhead:masterfrom
dobesv:nocache

Conversation

@dobesv
Copy link
Copy Markdown
Contributor

@dobesv dobesv commented Nov 2, 2011

This patch removes the in-memory caches. It also includes my improvements to support gzip since otherwise the two changes would conflict. Let me know if you'd like a non-gzip-included version of this.

Originally I considered fixing the caching issues I saw, such as:

  1. The cache is always populated with the static file result even if caching is disabled
  2. Items are never removed from the cache, so this may use a lot of memory
  3. In serveDir it checks if (pathname in exports.store) but runs streamFiles(exports.indexStore[pathname].files) (i.e. checking one cache but reading the other)

But I realized that users requiring in-memory caching should do this as part of a smarter middleware component or inside a proxy in front of node rather than leaning on the static files component to deal with it internally.

That way the memory overhead of the cache can be better controlled, monitored, and utilized by all components of the server (or cluster).

The cache as-is will be a kind of liability due to its unbounded size, and creating a full-featured cache might be more appropriate as its own project where dynamically output content within the node server can also benefit from it.

Thanks!

Dobes

…but adding .gz to the end can be served up to clients that support it. As part of that, changed the code so the Content-Type and Content-Length are sent even for an HTTP HEAD request.
…gth was sent with a 304 not modified response.
@dubiousdavid
Copy link
Copy Markdown
Contributor

Why was this pull-request never commented on? Seems like a lot of good stuff in here.

@phstc
Copy link
Copy Markdown
Collaborator

phstc commented Mar 30, 2013

It needs to be remerged.

This pull request cannot be automatically merged

@phstc phstc mentioned this pull request Apr 30, 2013
phstc added a commit that referenced this pull request Jun 28, 2013
Remove of in-memory cache, and check for gzipped files. Remerge of PR #36
@phstc phstc closed this Jul 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants