feat: Add MBTilesReloader (Tile Reload Phase 2)#2717
feat: Add MBTilesReloader (Tile Reload Phase 2)#2717CommanderStorm merged 35 commits intomaplibre:mainfrom
Conversation
CommanderStorm
left a comment
There was a problem hiding this comment.
Very clean. Good job.
I have a few comments (a bit more nit-picky than usual, but we are going to model the rest of this so I hope this is OK with you).
The biggest ask that I have:
- Please add a test in
tests/test.shthat auto-discovery works as intended with add, edit, move. - Please document this behaviour in our docs
Also, I don't nessarily think that we need an unstable-reloader feature..
LGTM if it works properly..
| let init: BTreeMap<String, (PathBuf, u64)> = match &config.mbtiles { | ||
| FileConfigEnum::Config(cfg) => { | ||
| let mut m = BTreeMap::new(); | ||
| if let Some(s) = &cfg.sources { | ||
| for (id, src) in s { | ||
| let path = src.get_path(); | ||
| let Ok(metadata) = path.metadata() else { | ||
| continue; | ||
| }; | ||
| let Ok(modified) = metadata.modified() else { | ||
| continue; | ||
| }; | ||
| let Ok(unix_epoch_delta) = modified.duration_since(UNIX_EPOCH) else { | ||
| continue; | ||
| }; | ||
|
|
||
| m.insert( | ||
| id.clone(), | ||
| (path.clone(), unix_epoch_delta.as_millis() as u64), | ||
| ); | ||
| } | ||
| } | ||
| m | ||
| } | ||
| _ => BTreeMap::new(), | ||
| }; |
There was a problem hiding this comment.
I think this collecting from the FS will be a relatively common pattern, at least for COG and PMTiles we will need to use something like this.
Lets extract the usefull part (checking one path? Very open to oppinions) to a method.
|
Thanks for the swift feedback @CommanderStorm! I'll get on this again some time this week. |
063bb6a to
3db3683
Compare
Split Catalog into StaticCatalog so the /catalog endpoint reads tile sources live from TileSourceManager, allowing hot-reloaded sources to appear without a restart. Simplify NewSource/DeletedSource by merging name + unique_name into a single pre-resolved id field, pushing ID resolution upstream. Improve MBTiles reloader bootstrap to compute initial source state with modification timestamps and handle all FileConfigEnum variants. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
`MartinError` was not `Send` because it contained `WebError(actix_web::Error)`, where `actix_web::Error` wraps a non-Send `dyn ResponseError`. This prevented `ReloadAdvisory` (which stores `MartinResult<BoxedSource>`) from being used across `.await` points inside `tokio::spawn`, as required by the MBTiles reloader. The `WebError` variant was introduced in f88fd10 but never used — no code converts an `actix_web::Error` into a `MartinError` via `?` or `.into()`. The srv layer returns `ActixResult` directly, not `MartinResult`. Also adds `Send + Sync` to the `MetricsIntialisationError` box (the only other non-Send variant), and updates the MBTiles reloader to use `MartinResult` and the now-async `apply_changes`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ad E2E tests Move watch-path and initial-source extraction from martin.rs into MBTilesReloader::new(), so the call site only hands over the raw config. Directories are now stored on the struct, removing the need to thread them through to watch() and discover_sources(). Extract the event-loop body into process_event() and update self.sources only after apply_changes() succeeds, avoiding stale state on error. Switch discover_sources() to async fs::read_dir and widen the timestamp field to u128 to preserve full millisecond precision. Add an integration test covering the full add/update/remove lifecycle for a watched MBTiles directory, including polling helpers for catalog and log assertions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cursive directory watching
…and tighten API Move TileSourceManager into the struct so callers don't need to thread it through to watch/start separately. Rename watch -> start to better reflect that it returns immediately after spawning a background task. Canonicalize watched directories eagerly in new() with explicit warnings on failure, so both start() and discover_sources() operate on the same resolved paths without redundant per-call canonicalization. Sort and dedup after canonicalization to avoid registering the same directory twice (which would produce duplicate reload events for a single file change). Remove the now-redundant is_absolute() filter in start() and the path.is_absolute() guard in discover_sources() — canonicalize() already guarantees absolute paths on success. Make discover_sources and process_event private and add doc comments to the two public methods. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n up discover_sources Adds `path_modified_ms` and `resolve_dir_entry` to `reload/mod.rs` so the per-entry warn-and-skip logic is reusable across future reloaders (cog, pmtiles, postgres). The initial source snapshot in `new()` and the hot-reload path in `discover_sources` both use these helpers, eliminating the duplicated metadata/timestamp chain. Also flattens the mbtiles extension guard to an early-continue and adds doc comments to the two private methods. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
c0611aa to
a578728
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This reverts commit fe582728fd27235071d9dffd0fe5e6ff4bac8fd0.
Remove the stale warning that catalog refresh at runtime is unsupported, and add a new section explaining the MBTiles hot reload feature introduced in this phase: watched directories automatically reflect file additions, modifications, and removals without a server restart. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… unused warning Use #[cfg(feature = "_tiles")] directly on the idr parameter of Config::resolve() and on the corresponding argument at each call site, so the parameter simply does not exist in feature combinations where it would be unused. Also restores the IdResolver import gates to _tiles across all affected files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CommanderStorm
left a comment
There was a problem hiding this comment.
Impressive work.
I have a few passing remarks, but let's merge
| - **File removed** — the source is removed from the catalog. | ||
|
|
||
| !!! note | ||
| Hot reload applies to directories configured under `mbtiles.paths` (or passed on the CLI). Named sources listed under `mbtiles.sources` are snapshotted at startup and are not watched for changes. |
There was a problem hiding this comment.
🤔 likely we can get rid of this by reshuffling some code.
Lets see if we can get rid of this.
Alas, I am fine with pushing this to a future PR or until someone has time to do this redhuffeling
There was a problem hiding this comment.
Yeah, my main hesitation to doing this now was that I don't know what the right way to determine the directories for watching should be. Right now, it's reasonable to assume if you configure a directory for Martin to gather sources from that you probably want Martin to also notice when new sources become available in that directory. With named sources, I still have the following questions about the right directories to watch:
- should we watch directories of the named sources?
- should we only watch the immediate parent of the source?
- if we watch more than the immediate parent, how far above the source should we watch?
| /// Tile Source Manager to which we should send `ReloadAdvisory` messages. | ||
| tile_source_manager: TileSourceManager, | ||
| /// Map of Source ID => (path, modified timestamp, cache policy). | ||
| sources: BTreeMap<String, (PathBuf, u128, CachePolicy)>, |
There was a problem hiding this comment.
nit: I usually prefer a struct for these things. Makes it clearer what the u128 is refering to for others, but I am fine as-is ^^
There was a problem hiding this comment.
Maybe can be addressed in the follow up to add support for the other local file tile sources.
|
CI needs to be made happy though.. |
Head branch was pushed to by a user without write access
…should_ be impossible as SQLite does not add FILE_SHARE_DELETE capability when opening the file
|
@CommanderStorm Windows is passing now (using a dirty hack). I wish we could simulate a real delete but it looks like SQLite has no way to add a FILE_SHARE_DELETE permission so faking the behaviour feels wrong but accurate since it shouldn't be possible on Windows. |
## 🤖 New release * `martin-tile-utils`: 0.7.0 -> 0.7.1 (✓ API compatible changes) * `mbtiles`: 0.17.0 -> 0.17.1 (✓ API compatible changes) * `martin-core`: 0.5.0 -> 0.5.1 (✓ API compatible changes) * `martin`: 1.7.0 -> 1.8.0 <details><summary><i><b>Changelog</b></i></summary><p> ## `mbtiles` <blockquote> ## [0.17.1](mbtiles-v0.17.0...mbtiles-v0.17.1) - 2026-04-28 ### Added - support `content_type` in PostgreSQL function source SQL comments for raster tiles ([#2671](#2671)) </blockquote> ## `martin-core` <blockquote> ## [0.5.1](martin-core-v0.5.0...martin-core-v0.5.1) - 2026-04-28 ### Other - update Cargo.toml dependencies </blockquote> ## `martin` <blockquote> ## [1.8.0](martin-v1.7.0...martin-v1.8.0) - 2026-04-28 ### Added - Add MBTilesReloader (Tile Reload Phase 2) ([#2717](#2717)) - support `content_type` in PostgreSQL function source SQL comments for raster tiles ([#2671](#2671)) ### Other - remove a few unused deps from Cargo.toml to not waste time building them ([#2738](#2738)) - update Cargo.toml dependencies </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
todo:
tests/test.shend to end test of moving files in and out