Skip to content

feat: Add retry mechanism on locked/busy mbtiles files#2572

Merged
CommanderStorm merged 7 commits intomaplibre:mainfrom
sdemeesterde:feature/retry-locked-mbtiles-files
Mar 4, 2026
Merged

feat: Add retry mechanism on locked/busy mbtiles files#2572
CommanderStorm merged 7 commits intomaplibre:mainfrom
sdemeesterde:feature/retry-locked-mbtiles-files

Conversation

@sdemeesterde
Copy link
Contributor

Fix #1591

Hello !
This is my first-ever PR to an open-source project.

Currently, martin crashes when trying to fetch the metadata of a busy mbtile file. It was proposed in the open issue: #1591 to be more lenient and perform retries when the file was locked.

Technical note:

The SQLITE_BUSY error is hardcoded as the sqlite wrapper used in martin (sqlx) doesn't offer an error enum match for specific sqlite error codes (unlike rusqlite).

Retry strategy:

The retrial strategy is an exponential backoff with the following delays: 50ms, 0.1s, 0.2s, 0.4s, 0.8s, 1.6s, 3.2s, and finally 6.4s.

I would happily take feedback on:

  • Is limiting the retry behavior strictly to SQLITE_BUSY errors the intended approach ?
  • Are the exponential backoff delays acceptable for this use case ?
  • Is the warning message too verbose for the logs ?
  • Is there anything else that I might have missed ? :)

Thanks !

Screenshot from 2026-02-24 14-09-21

@sdemeesterde sdemeesterde changed the title Add retry mechanism on locked/busy mbtiles files feat: Add retry mechanism on locked/busy mbtiles files Feb 24, 2026
Copy link
Member

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main feedback I have is to NEVER std::thread::sleep in an async context.
That is what tokio's sleep is for.
Since this is likely not avaliable under this feature already, you need to add this to Cargo.toml for the mbtiles feature.
Here is a resource that you might enjoy to learn more https://youtu.be/o2ob8zkeq2s

Also, could we use something like https://docs.rs/backon/1.6.0/backon/
This way we can have a simpler impl, but also a better impl (-> jitter+fibonacci).

@sdemeesterde
Copy link
Contributor Author

Thanks for the precious feedback ! I really appreciate that as I'm eager to learn more !
I actually had saved this video on my watch list, will watch it sooner I guess :)

@CommanderStorm CommanderStorm marked this pull request as draft February 27, 2026 20:36
@socket-security
Copy link

socket-security bot commented Mar 4, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcargo/​backon@​1.6.010010093100100

View full report

@sdemeesterde
Copy link
Contributor Author

Here is a version implementing your suggestion to use the backon crate.
Indeed, using Fibonacci with jitter is less aggressive while being robust !

(The "Decrusting the tokio crate" video was very interesting)

Copy link
Member

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@CommanderStorm CommanderStorm marked this pull request as ready for review March 4, 2026 20:54
Copilot AI review requested due to automatic review settings March 4, 2026 20:54
@CommanderStorm CommanderStorm enabled auto-merge (squash) March 4, 2026 20:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a retry/backoff mechanism when reading MBTiles metadata so Martin can tolerate transient SQLite SQLITE_BUSY (“database is locked/busy”) errors instead of failing immediately during source initialization (Fixes #1591).

Changes:

  • Add backon-based retry with Fibonacci backoff + jitter around mbt.get_metadata() for MBTiles sources.
  • Detect SQLITE_BUSY from the underlying sqlx error code to conditionally enable retries.
  • Add the backon dependency (workspace + martin-core feature wiring) and adjust Cargo.lock.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
martin-core/src/tiles/mbtiles/source.rs Adds SQLITE_BUSY detection and retries around MBTiles metadata fetching with logging on retry.
martin-core/Cargo.toml Enables backon (and tokio) under the mbtiles feature and adds optional dependency entry.
Cargo.toml Adds backon to workspace dependencies.
Cargo.lock Locks new transitive dependencies and updates some versions pulled in.
Comments suppressed due to low confidence (3)

martin-core/src/tiles/mbtiles/source.rs:61

  • There are two consecutive comments saying "Attempt to fetch metadata" here. Please remove the duplicate so the retry block has a single clear header comment.
        // Attempt to fetch metadata with backoff
        let start_delay = Duration::from_millis(50);

martin-core/src/tiles/mbtiles/source.rs:69

  • max_attempts is passed into FibonacciBuilder::with_max_times(...), which is easy to misread (times vs attempts). Consider renaming this variable to reflect what the backoff builder expects (e.g., max_times/max_retries) to avoid confusion when adjusting retry behavior later.
        let max_attempts = 10; // from 50ms to 2.75s
        let meta = (|| async { mbt.get_metadata().await })
            .retry(
                FibonacciBuilder::default()
                    .with_min_delay(start_delay)
                    .with_max_times(max_attempts)
                    .with_jitter(),
            )

martin-core/src/tiles/mbtiles/source.rs:77

  • The log message formats path.display() using {:?}; elsewhere in the codebase paths are logged with {} (e.g., martin-core/src/resources/sprites/mod.rs:80). Using {} here avoids debug-style quoting and keeps log formatting consistent.
                    "Database file {:?} locked (SQLITE_BUSY). Retrying in {:.2}s...",
                    path.display(),
                    dur.as_secs_f64()
                );

@CommanderStorm CommanderStorm merged commit 6b33822 into maplibre:main Mar 4, 2026
40 checks passed
@CommanderStorm CommanderStorm mentioned this pull request Mar 4, 2026
CommanderStorm added a commit that referenced this pull request Mar 15, 2026
## 🤖 New release

* `martin-tile-utils`: 0.6.10 -> 0.6.11 (✓ API compatible changes)
* `mbtiles`: 0.15.2 -> 0.15.3 (✓ API compatible changes)
* `martin-core`: 0.3.0 -> 0.3.1 (✓ API compatible changes)
* `martin`: 1.3.1 -> 1.4.0

<details><summary><i><b>Changelog</b></i></summary><p>


## `mbtiles`

<blockquote>

##
[0.15.3](mbtiles-v0.15.2...mbtiles-v0.15.3)
- 2026-03-14

### Added

- *(srv)* resolve some compression TODOS
([#2597](#2597))

### Fixed

- Accept any INT-containing type in MBTiles validation to be an integer
([#2560](#2560))

### Other

- rename the `webp.sql` fixture to `webp-no-primary.sql`
([#2564](#2564))
- More restrictive expects
([#2562](#2562))
</blockquote>

## `martin-core`

<blockquote>

##
[0.3.1](martin-core-v0.3.0...martin-core-v0.3.1)
- 2026-03-14

### Added

- Add retry mechanism on locked/busy mbtiles files
([#2572](#2572))

### Other

- More restrictive expects
([#2562](#2562))
- feature-gate PostgreSQL tests to remove external dependencies from
`cargo test` ([#2558](#2558))
</blockquote>

## `martin`

<blockquote>

##
[1.4.0](martin-v1.3.1...martin-v1.4.0)
- 2026-03-14

### Added

- *(srv)* resolve some compression TODOS
([#2597](#2597))
- Migrate mdbooks to zensical
([#2576](#2576))
- *(martin-cp)* indicativ based progress bar
([#2495](#2495))
- Add retry mechanism on locked/busy mbtiles files
([#2572](#2572))

### Fixed

- *(ui)* render MLT tiles correctly in Tile Inspector
([#2601](#2601))
- redirect ignoring `--route-prefix` for .pbf tile requests
([#2599](#2599))
- restrict zooming and panning on data inspector
([#2574](#2574))
- Accept any INT-containing type in MBTiles validation to be an integer
([#2560](#2560))

### Other

- *(deps)* Bump the all-npm-version-updates group across 2 directories
with 9 updates ([#2608](#2608))
- *(deps-dev)* Bump undici from 7.21.0 to 7.24.1 in /martin/martin-ui in
the all-npm-security-updates group across 1 directory
([#2602](#2602))
- *(deps)* autoupdate pre-commit
([#2592](#2592))
- *(deps)* Bump the all-npm-version-updates group across 2 directories
with 13 updates ([#2577](#2577))
- *(deps)* Bump the all-npm-security-updates group across 2 directories
with 1 update ([#2575](#2575))
- *(deps)* autoupdate pre-commit
([#2567](#2567))
- rename the `webp.sql` fixture to `webp-no-primary.sql`
([#2564](#2564))
- more cfg magic instead of #[allow(unused_variables)]
([#2563](#2563))
- More restrictive expects
([#2562](#2562))
- feature-gate PostgreSQL tests to remove external dependencies from
`cargo test` ([#2558](#2558))
</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>
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.

Retry locked mbtiles files

3 participants