-
Notifications
You must be signed in to change notification settings - Fork 281
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
Adding the MBTiles BlobStore #388
Conversation
|
||
MBTiles specification only supports JPEG and PNG formats and projection EPSG:3857 is assumed. The implemented blob store will read and write MBTiles files compliant with the specification but will also be able to write and read MBTiles files that use others formats and projections. | ||
|
||
Using the MBTiles blob store will bring several benefits at the cost of some performance loss. We will be able to store our data with a significantly smaller number of files, which will help us avoid file system headaches and help us managing our data. In some cases the stored data will be more compacted reducing the size of the data on disk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation tries not to use "we" or "you" but use a generic third person, this section should be reworked.
E.g. "The MBTiles storage uses a significantly smaller number of files, which results in easier data handling (e.g., backups, moving tiles between environments)".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compacted -> compact
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Just measured code coverage with the Eclipse EclEmma plugin, 75%, nicely done, it puts the module above the average code coverage in GWC. |
|
||
private static Log LOGGER = LogFactory.getLog(Utils.class); | ||
|
||
private static final ThreadLocal<String> LOG_ID = new ThreadLocal<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see nothing cleaning this thread local at the end of the request, this might cause cross request pollution and will make it harder to cleanly un-deploy the application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
I updated the pull request with your suggestions ... thanks for the deep and detailed review ! |
|
||
When performing a truncate of the cache the store will try to remove the whole database file avoiding to create fragmented space. This is not suitable for all the situations and is highly dependent on the database files granularity. The configuration property ``eagerDelete`` allows the user to disable or deactivate this feature which is disabled by default. | ||
|
||
When a truncate request by tile range is received all the the databases files that contains tiles that belong to the tile range are identified. If eager delete is set to true those databases files are deleted otherwise we perform a single delete query for each file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah hem, leftover "we" usage
Thread local gone, build is green... merging |
Associated proposal: https://github.com/GeoWebCache/geowebcache/wiki/MBTiles-BlobStore
This pull request incldues the blob store documentation.