Improve DB Indexes and Fix Deferred Index Logging in Compact DB Mode #245
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use "primary key without rowid" instead of "unique" on tiles_shallow (compact mode). This behind the scenes switches from a non-clustered to a clustered index. And this further reduces the DB and also improves read and write performance. Log the correct statement to manually create the index in compact DB mode. Move the automatic index creation into Mbtiles#createTables.
https://github.com/onthegomap/planetiler/actions/runs/2423265831 ℹ️ Base Logs f93e522ℹ️ This Branch Logs 26aa9ca |
It seems like the code evolved over time from deferring i.e adding the index after the inserts to skipping adding the index entirely and just logging the DDL statement to create the index.
msbarry
reviewed
Jun 1, 2022
| if (!config.deferIndexCreation()) { | ||
| mbtiles.addTileIndex(); | ||
| } | ||
| mbtiles.createTables(config.skipIndexCreation()); |
There was a problem hiding this comment.
One other thing I noticed in the mbtiles write benchmark is that it iterates through tile coordinates like this:
for (int z = 0; z <= 14; z++) {
int maxCoord = 1 << z;
for (int x = 0; x < maxCoord; x++) {
for (int y = 0; y < maxCoord; y++) {but the tile index order actually inverts Y - could you change the inner one to be:
for (int y = maxCoord - 1; y > 0; y--) {There was a problem hiding this comment.
good catch!
(assuming you meant for (int y = maxCoord - 1; y >= 0; y--) {)
msbarry
reviewed
Jun 1, 2022
| * manually | ||
| * @return {@code this} | ||
| */ | ||
| public Mbtiles createTables(boolean skipIndexCreation) { |
There was a problem hiding this comment.
it's a little hard to follow what the callers want with the true/false flag... could we make add a createTablesWithoutIndex so it's a bit more clear?
msbarry
reviewed
Jun 1, 2022
| TILES_COL_Y + ", " + TILES_COL_DATA + " blob);"); | ||
| // here "unique" is much more compact than a "primary key without rowid" because the tile data is part of the table | ||
| String tilesUniqueAddition = skipIndexCreation ? "" : """ | ||
| , unique(%s,%s,%s) |
There was a problem hiding this comment.
is there a difference between unique and primary key here?
There was a problem hiding this comment.
afaik no - other than the usual stuff that you can have only one PK and cannot insert NULL
=> let me change this to primary key since it expresses its purpose better
msbarry
reviewed
Jun 1, 2022
|
Kudos, SonarCloud Quality Gate passed! |
|
Looks good, thanks for implementing this! I'll get some performance and DB size tests on the planet along with the other performance improvements before putting out the next release. |
mmc1718
pushed a commit
to nmr8acme/planetiler
that referenced
this pull request
Jan 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
--defer-mbtiles-index-creationdoes not apply to--compact-db#243Here are some results on experimenting with PK with and without rowid, and unique on tiles and tiles_shallow