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

Remove sqlite index creation #453

Closed
otbutz opened this issue Jun 6, 2019 · 4 comments
Closed

Remove sqlite index creation #453

otbutz opened this issue Jun 6, 2019 · 4 comments

Comments

@otbutz
Copy link

otbutz commented Jun 6, 2019

// ensure indices exist
// note: this can be removed once the upstream PR is merged:
// https://github.com/whosonfirst/go-whosonfirst-sqlite-features/pull/4
new Sqlite3(dbPath)
.exec('CREATE INDEX IF NOT EXISTS spr_obsolete ON spr (is_deprecated, is_superseded)')
.close();

The mentioned PR has already been merged: whosonfirst/go-whosonfirst-sqlite-features#4

@missinglink
Copy link
Member

I just checked and unfortunately, that index is still not available in the sqlite files located at https://dist.whosonfirst.org/sqlite/.

I suspect there is a configuration problem on the server which generates the dist files, it possibly hasn't been updated:
whosonfirst/go-whosonfirst-sqlite-features#4 (comment)

orangejulius added a commit that referenced this issue Aug 2, 2019
The creation of the `spr_obsolete` index is problematic for the PIP
service and any of the admin lookup workers running in the importers.

Because there are always multiple processes doing admin lookup, and each
one tries to create the index if it doesn't exist, most end up failing.
SQLite only allows a single write lock on the DB, so all but one will
fail to achieve it.

However, in my testing, all it takes to solve this gracefully is to wrap
the index creation in a try/catch block. The index will be created by
one of the worker processes, and all subsequent queries appear to have
the performance improvements of the index (about 30-50% faster time to
load admin data).

Supercedes #431
Fixes #453
Connects #454 (we should not
actually remove the index creation quite yet)
orangejulius added a commit that referenced this issue Aug 2, 2019
The creation of the `spr_obsolete` index is problematic for the PIP
service and any of the admin lookup workers running in the importers.

Because there are always multiple processes doing admin lookup, and each
one tries to create the index if it doesn't exist, most end up failing.
SQLite only allows a single write lock on the DB, so all but one will
fail to achieve it.

However, in my testing, all it takes to solve this gracefully is to wrap
the index creation in a try/catch block. The index will be created by
one of the worker processes, and all subsequent queries appear to have
the performance improvements of the index (about 30-50% faster time to
load admin data).

Supersedes #431
Fixes #453
Connects #454 (we should not
actually remove the index creation quite yet)
orangejulius added a commit that referenced this issue Aug 2, 2019
The creation of the `spr_obsolete` index is problematic for the PIP
service and any of the admin lookup workers running in the importers.

Because there are always multiple processes doing admin lookup, and each
one tries to create the index if it doesn't exist, most end up failing.
SQLite only allows a single write lock on the DB, so all but one will
fail to achieve it.

However, in my testing, all it takes to solve this gracefully is to wrap
the index creation in a try/catch block. The index will be created by
one of the worker processes, and all subsequent queries appear to have
the performance improvements of the index (about 30-50% faster time to
load admin data).

Supersedes #431
Fixes #454
Connects #453 (we should not
actually remove the index creation quite yet)
@orangejulius
Copy link
Member

All the country-specific download files now have this index. The global file (whosonfirst-data-latest) still doesn't, but we are close to being able to remove this code.

@missinglink
Copy link
Member

missinglink commented Mar 18, 2020

This will be possible once #487 is merged, the spr_obsolete index is available in all Geocode Earth sqlite dist downloads.

@orangejulius
Copy link
Member

Finally done in #487. This importer now downloads data from builds sponsored by Geocode Earth, which are built with the spr_obsolete index. 🎉

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

No branches or pull requests

3 participants