Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LocalizedResourcePurger ttl configurable #339

Closed
rochoa opened this issue May 18, 2015 · 3 comments
Closed

LocalizedResourcePurger ttl configurable #339

rochoa opened this issue May 18, 2015 · 3 comments
Assignees
Milestone

Comments

@rochoa
Copy link
Contributor

rochoa commented May 18, 2015

The ttl is hard coded right now: https://github.com/CartoDB/Windshaft/blob/0.43.0/lib/windshaft/server.js#L382-L384

It should be started only for ttl > 0.

@rochoa rochoa self-assigned this May 18, 2015
@rochoa rochoa added this to the Alcobendas milestone May 18, 2015
@rochoa
Copy link
Contributor Author

rochoa commented May 22, 2015

I've been going a bit deep into the code and the problem with LocalizedResourcePurger is bigger than I originally thought.

LocalizedResourcePurger does not consider at all the possibility of long living renderer that does not uses some assets for a long period of time but at the same time it's not destroyed and at some point it can use the asset.

Imagine LocalizedResourcePurger is created with ttl set to 6, the following scenario is possible:

  • T0: Mapnik renderer created with two assets and some cartocss like
#layer {
  marker-file: url(https://example.com/icon.png);
  [zoom>9] {
    marker-file: url(https://example.com/icon10.png);
  }
}
  • T0-T6: Map keeps getting requests for levels in zoom 0..18.
  • T6: LocalizedResourcePurger runs and doesn't delete anything as it has been used
  • T7-T12: Map only gets requests in zoom 0..9.
  • T12: LocalizedResourcePurger runs and delete file for resource https://example.com/icon10.png as it was not used during the last ttl period.
  • T13: Map gets a request in zoom 10: BOOM!

Conclusion: without considering the in use renderers is not safe to delete cached resources.

@rochoa
Copy link
Contributor Author

rochoa commented May 22, 2015

My proposal is to remove LocalizedResourcePurger and don't look behind! 💃

One extra benefit of removing it is we could start caching XML for mapnik (with some changes).

@rochoa rochoa modified the milestones: Alcobendas, Castrillo Jun 1, 2015
@rochoa
Copy link
Contributor Author

rochoa commented Jun 5, 2015

Final decision: we are gonna remove LocalizedResourcePurger for now as it creates more issues than the problems it solves. We will build a better mechanism that will consider live renderers so their associated assets are not removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant