Skip to content

Implement pre-computed etag support for mbtiles sources#1918

Closed
Copilot wants to merge 4 commits intomainfrom
copilot/fix-1917
Closed

Implement pre-computed etag support for mbtiles sources#1918
Copilot wants to merge 4 commits intomainfrom
copilot/fix-1917

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jul 11, 2025

This PR implements more performant etag support by allowing mbtiles backends to supply pre-computed etags when available, eliminating unnecessary CPU usage from computing xxhash over tile data.

Background

CDNs rely heavily on etags (usually a hash) to identify if tiles have changed. Many .mbtiles files already store tiles with pre-computed hashes using two tables - one stores z, x, y, hash columns, and another table stores hash, data blob. Currently, Martin computes etags by running a non-cryptographical hash function over the output even when pre-computed hashes are available, causing unnecessary CPU usage.

Changes Made

1. Enhanced Source Trait

  • Added get_tile_etag() method with default implementation returning None
  • Added Sync bound for thread safety
  • Maintains full backward compatibility

2. Optimized MbtSource Implementation

  • Detects and stores MbtType during initialization
  • Implements get_tile_etag() to return pre-computed hashes for supported types:
    • MbtType::FlatWithHash: Uses hash from tiles_with_hash table
    • MbtType::Normalized: Uses hash from images table tile_id
    • MbtType::Flat: Returns None (falls back to computed hash)

3. Updated Tile Serving Pipeline

  • Extended Tile struct with optional etag field
  • Modified tile serving logic to use pre-computed etag for single-source requests
  • Updated compression/decompression functions to preserve etag
  • Falls back to xxhash computation when no pre-computed etag is available

4. HTTP Response Integration

  • Modified get_http_response() to prefer pre-computed etag over computed hash
  • Maintains identical HTTP behavior - completely transparent to clients

Performance Impact

  • For mbtiles with pre-computed hashes: Eliminates xxhash computation entirely - instant etag lookup
  • For flat mbtiles or other sources: No performance change - same xxhash computation as before
  • Memory overhead: Minimal - only adds optional String to Tile struct
  • Network behavior: Identical - etag headers work exactly the same

Testing

Added comprehensive tests verifying:

  • Pre-computed etag functionality with real mbtiles files
  • HTTP responses correctly use pre-computed etags
  • Backward compatibility - all existing tests continue to pass
  • Sources without pre-computed etags work normally

Backward Compatibility

Fully backward compatible - no breaking changes

  • Existing sources (PostgreSQL, PMTiles, etc.) continue working exactly as before
  • Flat mbtiles files continue working with computed etags
  • All existing functionality preserved

Example Usage

// For FlatWithHash or Normalized mbtiles:
let etag = source.get_tile_etag(xyz, None).await?;
// Returns Some("80EE46337AC006B6BD14B4FA4D6E2EF9") - pre-computed hash

// For Flat mbtiles or other sources:
let etag = source.get_tile_etag(xyz, None).await?;
// Returns None - will compute xxhash as before

The implementation successfully addresses the performance bottleneck described in the issue while maintaining full compatibility with existing functionality.

Fixes #1917.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI and others added 2 commits July 11, 2025 02:35
Co-authored-by: nyurik <1641515+nyurik@users.noreply.github.com>
Co-authored-by: nyurik <1641515+nyurik@users.noreply.github.com>
Copilot AI changed the title [WIP] More performant etag support Implement pre-computed etag support for mbtiles sources Jul 11, 2025
Copilot AI requested a review from nyurik July 11, 2025 02:42
@CommanderStorm
Copy link
Copy Markdown
Member

For what this PR is worth, I think this is simpler if re-done by a human.

@nyurik
Copy link
Copy Markdown
Member

nyurik commented Jul 11, 2025

I agree that this should be done by a human. The goal was to see how this system works in general, and if we can use it to solve simpler tasks

@CommanderStorm
Copy link
Copy Markdown
Member

It definitively can - just in rust the quality of code it produces is not great.

In react though..

@CommanderStorm CommanderStorm deleted the copilot/fix-1917 branch September 14, 2025 16:38
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

Successfully merging this pull request may close these issues.

Less CPU intensive etag implementation

3 participants