Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Offline (mbgl-core implementation) #3715

Merged
merged 20 commits into from
Feb 11, 2016
Merged

Offline (mbgl-core implementation) #3715

merged 20 commits into from
Feb 11, 2016

Conversation

jfirebaugh
Copy link
Contributor

TODO list:

  • Normalize Mapbox tile URLs to mapbox://tiles scheme before requesting, translate back to api.mapbox.com URLs inside OnlineFileSource.
  • Remove sqlite_cache.{hpp,cpp}
  • Style, source, sprite, glyphs downloading
  • Figure out progress semantics when complete file count is indeterminate
  • Font stack normalization
  • Verify that LatLngBounds enforces the correct invariants: LatLng and LatLngBounds should enforce invariants #3802
  • Tiles downloading
  • Tile ID wrapping
  • Region deletion
  • Test cases
    • No sprite
    • No glyphs
    • Inline source
    • GeoJSON source
  • Size reporting
  • Remove schema version logic; once shipped we need to write migrations rather than dumping existing dbs
  • Review use of Request::Error::NotFound
  • Get status of inactive download
  • Error handling
  • Resource deletion
  • Concurrency
  • Performance
  • Size limits / eviction
  • Polish schema
  • Consistently use ?NNN syntax for prepared statements

Think about

  • Style spec migrations -- After we release v9, we'll need to continue supporting offline databases containing v8 styles, via version branches in the renderer, or JIT migration.
  • Static evaluation of font stacks vs. data driven styling -- The required font stacks need to be determined without loading data. Therefore we should either not support data-driven "text-font", or require that all possible font stacks values be listed elsewhere, like we do with "icon-image".
  • Stop tile pyramid recursion when encountering a nodata tile -- This is an potential future optimization to avoid storing nodata child tiles once we encounter a nodata parent. This depends on Overzoom cached/offline tiles #123.

* Under the hood, SQLite creates surrogate keys (ROWID) anyway. We may as well take advantage of this and use the surrogates for foreign keys as well, since they are simpler and more efficient than compound foreign keys.
* Create indexes for efficient eviction queries
This results in a ~1% increase in database size, which is worth it for reducing complexity and making the tiles and resources tables more similar in structure.
@jfirebaugh
Copy link
Contributor Author

Notes from the "Think about" section:

  • Style spec migrations -- After we release v9, we'll need to continue supporting offline databases containing v8 styles, via version branches in the renderer, or JIT migration.
  • Static evaluation of font stacks vs. data driven styling -- The required font stacks need to be determined without loading data. Therefore we should either not support data-driven "text-font", or require that all possible font stacks values be listed elsewhere, like we do with "icon-image".
  • Stop tile pyramid recursion when encountering a nodata tile -- This is an potential future optimization to avoid storing nodata child tiles once we encounter a nodata parent. This depends on Overzoom cached/offline tiles #123.

@jfirebaugh jfirebaugh removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Feb 11, 2016
@jfirebaugh jfirebaugh changed the title Offline Offline (mbgl-core implementation) Feb 11, 2016
@jfirebaugh jfirebaugh merged commit 20be32f into master Feb 11, 2016
@jfirebaugh jfirebaugh deleted the offline branch February 11, 2016 00:42
@bsudekum
Copy link

Congrats 🎉!

@robmaceachern robmaceachern mentioned this pull request Mar 17, 2017
kkaefer added a commit that referenced this pull request Jun 12, 2018
Before this change, we've tried to open the database in read/write, but not create mode. In situations where the database didn't exist yet, this logged an error to the console, and we proceeded to opening it again in read/write/create mode, which actually created the file. The reason we did this is so that we could detect really old caching databases from January 2016 in case a developer upgraded from an older SDK (iOS v3.1 and earlier, Android v3.2 and earlier) that didn't have #3715 yet.

However, these error messages, while innocent, look scary in the console and some users suspect that it's a bug. This change opens the file directly in read/write/create mode, omitting the first failed attempt. To handle old cache databases, we're now deleting the `http_cache` table, which was the only table in those old databases, and create the new schema, rather than deleting the entire file and recreating the Database object. In most scenarios, this will lead to one fewer opening attempt, while the database migration will continue to work for the few users who upgrade all the way from a January 2016 SDK.

Additionally, this fixes a mismatch between the Qt and non-Qt implementation: Qt doesn't support opening a file in read/write mode without the create flag. This means that we've seen a different control flow on Qt compared to the non-Qt implementation when opening a database.
kkaefer added a commit that referenced this pull request Jun 12, 2018
Before this change, we've tried to open the database in read/write, but not create mode. In situations where the database didn't exist yet, this logged an error to the console, and we proceeded to opening it again in read/write/create mode, which actually created the file. The reason we did this is so that we could detect really old caching databases from January 2016 in case a developer upgraded from an older SDK (iOS v3.1 and earlier, Android v3.2 and earlier) that didn't have #3715 yet.

However, these error messages, while innocent, look scary in the console and some users suspect that it's a bug. This change opens the file directly in read/write/create mode, omitting the first failed attempt. To handle old cache databases, we're now deleting the `http_cache` table, which was the only table in those old databases, and create the new schema, rather than deleting the entire file and recreating the Database object. In most scenarios, this will lead to one fewer opening attempt, while the database migration will continue to work for the few users who upgrade all the way from a January 2016 SDK.

Additionally, this fixes a mismatch between the Qt and non-Qt implementation: Qt doesn't support opening a file in read/write mode without the create flag. This means that we've seen a different control flow on Qt compared to the non-Qt implementation when opening a database.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants