make FS cache re-check with server once per week

Bug #579442 reported by Will Uther
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenSatNav
Fix Released
Medium
Guillaume Rosaire

Bug Description

Normally when you think of a web cache, you imagine that you get a fast load time from local storage, but that at some point the cache will expire and you'll check against the server for updated information. Under the old tile cache code this was never happening: if the FS contained a tile then it would be displayed. This meant that any updates you made to OSM would never make it to your OSN display.

This patch fixes that problem. It updates the DB scheme to also store the date the tile was last checked against the server. If that was less than a week ago, then the cached tile is used. If it was more than a week ago, then the cached tile is displayed immediately, and the server is checked for an updated tile in the background. When the server responds, the disk cache is updated and then the display is updated.

This patch introduces a slight flicker when an old tile has already been displayed and it is updated to a new tile. Its minor and only happens once per week for any given tile.

As I only wrote this today, I obviously haven't tested it for a week. :) I did force some of the tests to true to test other code paths, but I may have missed something.

Revision history for this message
chris_debian (cjhandrew) wrote :

This is something I've mentioned on the lists and is a welcome addition.

Thanks,

Chris.

Revision history for this message
Guillaume Rosaire (zerog) wrote :

If the user has no data plan, will it annoy him ? Will it fail gracefully ? Will he have any error ?

Should the one week delay be defined in a preference screen or not ?

Revision history for this message
Will Uther (willu-mailinglists) wrote :

Can an app detect the users data plan? Or only whether they have a network connection?

If you were writing code, would you write a whole new downloader, or would you use the one already there?

Does the one already there fail gracefully or does the user get an error?

Have you read http://wiki.openstreetmap.org/wiki/Slippy_Map to see where the 7 day limit might come from?

Why are we having this discussion in questions?

Revision history for this message
Guillaume Rosaire (zerog) wrote :

Sorry if my questions seemed rude, that was just genuine questions I have. And I know that several users are trying to use map tilepacks and offline routing, for they don't have data plans, I was wondering if that could be an issue.

English is not my native language, I didn't at all try to bother you.

Revision history for this message
Will Uther (willu-mailinglists) wrote :

ZeroG - I didn't think your questions were rude :). I phrased my response as questions for 2 reasons:

  a) I was just having some fun, and
  b) the teacher in me was trying to get you to answer them for yourself.

I should have put more smilies in. Sorry.

In case the answers still aren't clear:
 - I'm one of the users who has no data plan, so that is a use case I am quite interested in,
 - There is no way to tell if the user has a data plan. The system can only tell if there is a net connection.
 - With this patch, any tiles cached on disk are displayed before an attempt to download is made
 - If there is no net connection then the attempt to download an updated tile will fail. There will be an extra log message, but otherwise this will be transparent to the user.

Revision history for this message
Guillaume Rosaire (zerog) wrote :

Well, I misread your last sentence (I read Why are we having this discussion - without the important part 'in questions'), and didn't see it was a joke.

Anyway, as you're in the no or low data plan case, I guess you've sorted all this out.

Regards
Guillaume

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

I think this idea is good but what about 1 more addition: Store the Etag generated by the OSM server into the DB then send that with the tile request. I checked in Firefox and the OSM server correctly sends a '304 Not Modified' header when given an 'If-None-Match'. The OSM server sends an Expires header but it is only about 6 hours into the future which I think is too short so I'm happy with a week. This will definitely help users on a low data plan (I'm on 300Mb/month).

Revision history for this message
Will Uther (willu-mailinglists) wrote :

What is an 'Etag'? Ahhh... http://en.wikipedia.org/wiki/HTTP_ETag

That looks like a fine idea. I'm not sure when I'll get to it (busy at work again). There is sample code here: http://www.oreillynet.com/onjava/blog/2004/07/optimizing_http_downloads_in_j.html .

I currently try to add an 'ifModifiedSince:' header, but I'm not sure I'm doing it right as I never see any 'HTTP_NOT_MODIFIED' responses. I don't know if OSM handles that header. I haven't tried to packet sniff to check things are ok. The sample code above also adds an ifModifiedSince header, but differently to the way I do it.

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

Yeah, the OSM server doesn't send last modified times so don't worry about trying that :) Etags serve the same purpose so it sort of makes sense that they just picked one way.

Revision history for this message
chris_debian (cjhandrew) wrote :

Will,

Have you had a chance to have a look at this? Just wondering how it's progressing.

Cheers,

Chris.

Revision history for this message
Will Uther (willu-mailinglists) wrote :

I'm swamped at work right now. I'm not sure when I'll get a chance to look at this (or any other OSN stuff).

Revision history for this message
chris_debian (cjhandrew) wrote :

willu wrote:
> I'm swamped at work right now. I'm not sure when I'll get a chance to look at this (or any other OSN stuff).

No worries, Willu. Should this be re-assigned? Any preferences?

Cheers,

Chris.

Revision history for this message
Guillaume Rosaire (zerog) wrote :

The Etag part shouldn't be too hard to implement, I'll give it a try.

Revision history for this message
chris_debian (cjhandrew) wrote :

Many thanks, be good to move this forward.

Chris.

Revision history for this message
Guillaume Rosaire (zerog) wrote :

While implementing the etag thing I spotted a bug in incrementUse method in OpenStreetMapTileFilesystemProvider.java line 361

<pre>
                public void incrementUse(final String aFormattedTileURLString) {
                        final Cursor c = this.mDatabase.rawQuery("UPDATE " + T_FSCACHE
                                        + " SET " + T_FSCACHE_USAGECOUNT + " = "
                                        + T_FSCACHE_USAGECOUNT + " + 1 , " + T_FSCACHE_TIMESTAMP
                                        + " = '" + getNowAsIso8601() + "' WHERE " + T_FSCACHE_NAME
                                        + " = '" + aFormattedTileURLString + "'", null);
                        c.close();
                }
</pre>

On my device, this query doesn't update anything, I had to change the line for a this.mDatabase.execSQL(query)

That means that the timestamp of last access and countuse fields were never updated/incremented.

I'll fix this in my etag commit (I need to debug a few thing before, but that should arrive quickly).

Now that these fields are correctly updated, that could help with the policy used for least used or oldest tiles to be deleted.

Revision history for this message
chris_debian (cjhandrew) wrote :

Well spotted!

That should help.

Chris.

Revision history for this message
Guillaume Rosaire (zerog) wrote :

Patch commited in revision 98.

With :
Added : ETag support in HTTP downloads
Various little fixes
SQLite DB model updated with 3 fields used_timestamp, added_timestamp and etag. (DB version 5 to have it upgraded on tester devices at first launch)
new class TileMetadata, bean with last ETag, data of last download for each tile, mapped with the DB - We could add in this bean the tile URL, that would remove one more parameter when calling the getTile()/loadTile() methods.
incrementUse method fixed, it now correctly updates the tile table.

I had a strange behavior when testing from time to time : when downloading tiles, OSN try to save them as files in internal memory rather than on the SD Card, and when this is happening, it never manage to save them, the tiles are always shown as 'Loading'. Please tell me if you encounter the same issue, for I don't understand why this is happening (I don't think I modified anything relating to this)

Revision history for this message
chris_debian (cjhandrew) wrote :

ZeroG wrote:
> Patch commited in revision 98.
>
> With :
> Added : ETag support in HTTP downloads
> Various little fixes
> SQLite DB model updated with 3 fields used_timestamp, added_timestamp and etag. (DB version 5 to have it upgraded on tester devices at first launch)
> new class TileMetadata, bean with last ETag, data of last download for each tile, mapped with the DB - We could add in this bean the tile URL, that would remove one more parameter when calling the getTile()/loadTile() methods.
> incrementUse method fixed, it now correctly updates the tile table.
>
> I had a strange behavior when testing from time to time : when downloading tiles, OSN try to save them as files in internal memory rather than on the SD Card, and when this is happening, it never manage to save them, the tiles are always shown as 'Loading'. Please tell me if you encounter the same issue, for I don't understand why this is happening (I don't think I modified anything relating to this)

ZeroG,

Can we do anything more with this? What's the way forward?

Thanks,

Chris.

Revision history for this message
Guillaume Rosaire (zerog) wrote :

That must come from the fact that I plugged/unplugged my handset while testing, and each time, the SD card was checked for error by Android. During that time, the SD card was unavailable and tiles were not saved.

May be related to bug #76

Revision history for this message
chris_debian (cjhandrew) wrote :

Guillaume,

Thanks for the update. What do we need to do before this issue is closed?

Chris.

Revision history for this message
Guillaume Rosaire (zerog) wrote :

I'm just waiting for feedback after people test it for a few days. At the moment, I feel like the case can be closed.

Revision history for this message
chris_debian (cjhandrew) wrote :

Brilliant! That'll be a step closer to 0.9.

Thanks,

Chris.

Revision history for this message
Kieran Fleming (kieran-fleming) wrote :

I'm happy with this so I'll close it.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.