Set etag on pmtiles serve responses #137
Merged
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
msbarry
commented
Feb 10, 2024
main.go
Outdated
| @@ -88,6 +89,7 @@ var cli struct { | |||
| CacheSize int `default:"64" help:"Size of cache in Megabytes."` | |||
| Bucket string `help:"Remote bucket"` | |||
| PublicURL string `help:"Public base URL of tile endpoint for TileJSON e.g. https://example.com/tiles/"` | |||
| TileEtag bool `help:"Generate etag for each tile instead of using archive etag"` | |||
There was a problem hiding this comment.
The way I implemented this it also applies to metadata json and tilejson responses. Should I remove those? Or make this --resource-etag? or just leave it as-is?
There was a problem hiding this comment.
Also in my tests on a 32-core machine enabling this reduces throughput from 40k tiles per second to 36k tiles per second.
There was a problem hiding this comment.
maybe --unique-etags instead, to make it clearer the effect?
msbarry
commented
Feb 10, 2024
msbarry
commented
Feb 11, 2024
bdon
reviewed
Feb 13, 2024
| @@ -294,6 +296,7 @@ func (server *Server) getTileJSON(ctx context.Context, httpHeaders map[string]st | |||
| } | |||
|
|
|||
| httpHeaders["Content-Type"] = "application/json" | |||
| httpHeaders["Etag"] = generateEtag(tilejsonBytes) | |||
There was a problem hiding this comment.
I think the end result is case-insensitive but the capitalization convention seems to be ETag with a capital T, any objections to changing it to that?
bdon
added a commit
that referenced
this pull request
Feb 13, 2024
bdon
added a commit
that referenced
this pull request
Feb 13, 2024
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.
Set etag on tile, metadata, and tilejson responses. By default it uses the etag from the archive but you can opt into computing an etag per-resource with
--tile-etagflag. The default archive etag behavior minimizes runtime overhead and is guaranteed to return a new etag when tile contents changes, but also ends up changing the etag (and purging intermediate caches) when the archive changes but an individual tile is the same between old and new archive.This also implements
If-MatchandIf-None-Matchheader handling by using go's built-inhttp.ServeContentutility. As a side effect this also means thatpmtiles servenow supports range requests within individual tiles.