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

Reduce UTFGrid Tile Caching #3087

Closed
wants to merge 2 commits into from

Conversation

rajadain
Copy link
Member

Overview

Attempts to help with incorrect UtfGrid results by appending a random string to tile requests to get fresh tiles every time. Unfortunately, because of CartoDB/Windshaft#310, the true underlying issue can only be fixed by upgrading Windshaft, which can be a pretty involved process (see OpenTreeMap/otm-tiler#116).

We also disable the drawing tool upon search, hoping to create a further delay that could help invalidate the internal cache.

Connects #3082

Notes

This is not a new bug, and has probably been there from the very beginning. A short term solution would be to invalidate all .grid.json caches in production when this is deployed.

Testing Instructions

  • Check out this branch and bundle
  • Go to :8000 and select a boundary to draw.
  • Before picking a boundary, search for a new location that takes you to a different part of the country
    • Ensure that the draw tool is closed
  • Enable the same boundary draw tool again. Hover over the new location area.
    • Ensure you get hover values for the new location, and not the old one

Because of CartoDB/Windshaft#310, UtfGrid tiles from the
server can be incorrect due to internal caching. These
incorrect results can be cached by the CDN and in browsers
for up to 30 days, making it difficult to use the app.

By appending a random querystring to the end of each tile
request for UtfGrid values, we try and bust the cache and
attempt to retrieve fresh values from the server every time,
in the hopes that they will be correct.
Previously, if the user were to geocode search for something
while a drawing tool was open, it would stay open when they
landed in the new location. Now we close it, to force them to
start drawing fresh for every location.
@rajadain rajadain added the NSF Funding Source: National Science Foundation label Feb 20, 2019
@rajadain rajadain changed the base branch from develop to release/1.24.2 February 20, 2019 18:53
Copy link
Contributor

@mmcfarland mmcfarland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in slack (https://azavea.slack.com/archives/C044CG212/p1550687631002300), this likely won't have the intended effect. Once the tile is initially rendered and placed on s3, query string parameters won't have an effect on subsequent requests. Since the problem lies within an internal cache of windshaft, the query string param would never get passed into the server and bust the cache.

Since our development environments bypass the s3 layer, this appears to work locally but would fail to fix the issue in staging/prod.

@@ -231,6 +231,8 @@ var DrawWindow = Marionette.LayoutView.extend({
model: this.model,
resetDrawingState: resetDrawingState
}));

this.listenTo(App.map, 'change:lat, change:lng', resetDrawingState);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be having a wider impact than intended, for other workflows. For example, if you geocode within the same city, the map moves only slightly, but the loaded boundary layer is removed. Was this meant to be targeted only at the free-hand draw tool?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I think this is too wide-ranging. I think it makes sense to do this for both free-draw and boundary, especially if it helped with the UtfGrid caching issue, but since it doesn't I don't think we should keep it. I'm going to close this PR.

@rajadain rajadain closed this Feb 21, 2019
@rajadain rajadain deleted the tt/reduce-utfgrid-tile-caching branch June 12, 2020 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NSF Funding Source: National Science Foundation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants