developmentseed / developmentseed/titiler-covjson

Count the band axis in `max_samples`: an expression multiplies the values per position

Open
#103 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
1
Forks
1
Avg merge
2d 1h
Merged PRs (30d)
12

Description

`max_samples` caps the number of positions a `MULTIPOINT` may name, one point read each. It counts positions only, and a caller can multiply the work per position with `expression`: each `;`-separated block is evaluated into its own band, so N positions with M blocks produce N x M values from a cap that only saw N.

Split out of #100, which counted the band axis in `max_cells` for `/bbox` and `/area`. It deliberately did not extend to here: each sample is a single cell, so there is no large allocation to bound and the resource is different. Per [ADR-0006](https://github.com/developmentseed/titiler-covjson/blob/main/docs/adr/0006-resource-ceilings-bound-the-resource.md), a ceiling should bound the resource actually consumed, which means this needs its own reasoning rather than a copy of the `max_cells` fix.

## Measured

On a default factory (`max_samples=1000`), against a 1-band source:

| positions | expression blocks | values returned |
| --- | --- | --- |
| 1000 | 1 | 1,000 |
| 1000 | 50 | 50,000 |

CPU time and response size both scale linearly in positions x blocks, and blocks are uncapped at a few characters each, so 1000 blocks is ~1M values in a single JSON response. The response size may be the more interesting half: `max_samples` reads as a bound on how large a `/position` response can get, and it is not one.

## Priority

Low. There is no memory spike, the cost is linear rather than quadratic, and a deployer who lowers `max_samples` lowers this with it. Worth fixing because the knob does not mean what it appears to mean, not because the exposure is severe.

## Notes for whoever picks this up

Decide which quantity the ceiling counts before picking its shape. ADR-0006's rule does not settle that on its own here, since the growth appears as reads, CPU, and response size at once. Candidates to weigh:

- Bound `positions x bands` against `max_samples`, mirroring `max_cells`. Simple and consistent, but it changes what an existing `max_samples` value permits, and "samples" then means value reads rather than positions.
- Bound the value count separately, leaving `max_samples` a position cap. Honest to the name, adds a fourth knob.
- Bound the expression block count globally, which would also cover `/position` with a single `POINT` (currently unbounded, though trivially so at one cell).

Note the zero-band case that #100 turned up: an expression yielding no blocks makes any `x bands` product zero. There it was safe because rio-tiler rejects the expression before allocating; confirm that holds on the point path too rather than assuming it.

Contributor guide

No contributing guide indexed for this repository

Research direction

Read docs/adr/0006-resource-ceilings-bound-the-resource.md, then trace the /position point path and its MULTIPOINT handling. Verify how rio-tiler treats an expression with zero blocks, and compare the measured positions-versus-expression-blocks behavior. Done means the chosen ceiling matches the intended resource and is covered for the noted cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.