mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-11-12 23:31:02 +00:00
fix(maps): keep a slow Photon from holding back the suggestions
The suggestions waited for Photon beside the index for up to its full 2.5 s deadline on every keystroke, and their misses and spent allowance paused Photon for the search and the reverse lookup as well. Once the index has answered, Photon now gets 300 ms more and is then called off; with nothing from the index it is still waited for. The keystroke question has a shorter deadline and its own tally of misses and pauses, a question called off by the list counts as no miss, and a pause of the search still holds the suggestions back. Refs #2877
This commit is contained in:
@@ -351,6 +351,22 @@ export function mergeSuggestions(fromIndex: Suggestion[], fromPhoton: Suggestion
|
||||
return interleave(fromIndex, extra, limit);
|
||||
}
|
||||
|
||||
/** How long Photon's suggestions may still take once the index has answered. */
|
||||
export const PHOTON_GRACE_MS = 300;
|
||||
|
||||
/** What `pending` answers within `ms`, or null when it takes longer than that. */
|
||||
export async function answerWithin<T>(pending: Promise<T | null>, ms: number): Promise<T | null> {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const late = new Promise<null>((resolve) => {
|
||||
timer = setTimeout(() => resolve(null), ms);
|
||||
});
|
||||
try {
|
||||
return await Promise.race([pending, late]);
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The details record of a place picked out of the index, with what
|
||||
* OpenStreetMap knows about the same building filled in.
|
||||
|
||||
@@ -5,6 +5,7 @@ import type { GeoLane } from '../geo/nominatim.client';
|
||||
import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service';
|
||||
import type { ApiKeySource } from '../settings/instance-api-keys';
|
||||
import {
|
||||
answerWithin,
|
||||
belonging,
|
||||
indexPoiAnswer,
|
||||
indexPoiPlan,
|
||||
@@ -12,6 +13,7 @@ import {
|
||||
indexSuggestions,
|
||||
mergeIndexPages,
|
||||
mergeSuggestions,
|
||||
PHOTON_GRACE_MS,
|
||||
} from './maps-index.helpers';
|
||||
import { NEARBY_DEFAULT_LIMIT, NEARBY_DEFAULT_RADIUS_M, nearbyCacheKey, nearestFirst } from './maps-nearby.helpers';
|
||||
import { MapsUrlResolver, type ResolvedMapsUrl } from './maps-url.resolver';
|
||||
@@ -557,22 +559,25 @@ export class MapsService {
|
||||
// Photon beside them (#2877). The layer is a slice of OpenStreetMap, and
|
||||
// a place missing from it showed up only once the search was sent.
|
||||
// Photon is built for this keystroke and answers null when the services
|
||||
// are off, so the pair costs the slower one and an install without
|
||||
// Photon reads exactly as before. Each side swallows its own failure, so
|
||||
// one that is down does not throw away what the other found.
|
||||
const [found, photon] = await Promise.all([
|
||||
trekPlacesSearch(input, {
|
||||
lat: boxCentre?.lat,
|
||||
lng: boxCentre?.lng,
|
||||
limit: 8,
|
||||
sources: 'index,osm',
|
||||
lang,
|
||||
}).catch((err: unknown) => {
|
||||
console.warn('TREK Places autocomplete failed, falling back:', (err as Error).message);
|
||||
return [];
|
||||
}),
|
||||
this.osm.autocompletePhoton(input, lang, boxCentre),
|
||||
]);
|
||||
// are off, so an install without Photon reads exactly as before. Each
|
||||
// side swallows its own failure, so one that is down does not throw away
|
||||
// what the other found. Photon is the extra here, so once the index has
|
||||
// answered it gets a moment more and is then dropped: a slow Photon must
|
||||
// not hold back what the index already has.
|
||||
const typing = new AbortController();
|
||||
const asked = this.osm.autocompletePhoton(input, lang, boxCentre, typing.signal);
|
||||
const found = await trekPlacesSearch(input, {
|
||||
lat: boxCentre?.lat,
|
||||
lng: boxCentre?.lng,
|
||||
limit: 8,
|
||||
sources: 'index,osm',
|
||||
lang,
|
||||
}).catch((err: unknown) => {
|
||||
console.warn('TREK Places autocomplete failed, falling back:', (err as Error).message);
|
||||
return [];
|
||||
});
|
||||
const photon = found.length > 0 ? await answerWithin(asked, PHOTON_GRACE_MS) : await asked;
|
||||
typing.abort();
|
||||
const suggestions = mergeSuggestions(indexSuggestions(found, lang), photon ?? []);
|
||||
if (suggestions.length > 0) {
|
||||
return { suggestions, source: found.length > 0 ? 'trek-places' : 'openstreetmap' };
|
||||
|
||||
@@ -529,10 +529,11 @@ export class OsmClient {
|
||||
/**
|
||||
* Suggestions from Photon, which is built for search-as-you-type, or null when
|
||||
* it is not in use or did not answer. Never Nominatim in its place: its usage
|
||||
* policy rules out autocomplete.
|
||||
* policy rules out autocomplete. Aborting `signal` drops a question nobody
|
||||
* waits for any more.
|
||||
*/
|
||||
async autocompletePhoton(input: string, lang?: string, bias?: { lat: number; lng: number }) {
|
||||
const found = await this.photon.search(input, { lang, bias });
|
||||
async autocompletePhoton(input: string, lang?: string, bias?: { lat: number; lng: number }, signal?: AbortSignal) {
|
||||
const found = await this.photon.suggest(input, { lang, bias, signal });
|
||||
return found?.map(asSuggestion).filter((row) => row !== null) ?? null;
|
||||
}
|
||||
|
||||
|
||||
@@ -14,6 +14,12 @@
|
||||
* A spent allowance says itself how long to wait (Retry-After), and that wait
|
||||
* is kept instead.
|
||||
*
|
||||
* The suggestions asked on every keystroke keep a tally of their own. They
|
||||
* come far more often than a search, under a shorter deadline, and many are
|
||||
* dropped unanswered once the next keystroke is in, so their misses and their
|
||||
* spent allowance pause only the suggestions. A pause of the search holds the
|
||||
* suggestions back too: a host the search cannot reach is not asked per letter.
|
||||
*
|
||||
* The key comes from serviceIdentitySlot, as it does for the Places request
|
||||
* (trek-places.fetch.ts), so the maps domain reads the installation identity
|
||||
* one way and needs no import of ServiceIdentityModule; the slot holds the
|
||||
@@ -33,6 +39,8 @@ import type { ConfigType } from '@nestjs/config';
|
||||
|
||||
/** Short, because Nominatim still has to fit behind it inside the browser's wait. */
|
||||
const TIMEOUT_MS = 2500;
|
||||
/** The suggestion list is waiting on the index beside it, and the next keystroke is near. */
|
||||
const SUGGEST_TIMEOUT_MS = 1500;
|
||||
/** Ten hits are a few kB. */
|
||||
const MAX_BYTES = 512_000;
|
||||
const SEARCH_LIMIT = 10;
|
||||
@@ -51,22 +59,33 @@ interface PhotonSearchOptions {
|
||||
bias?: { lat: number; lng: number };
|
||||
}
|
||||
|
||||
/** The misses in a row and the pause of one kind of question. */
|
||||
interface Tally {
|
||||
/** What answers instead while Photon does not. */
|
||||
fallback: string;
|
||||
misses: number;
|
||||
pausedUntil: number;
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class PhotonClient {
|
||||
private misses = 0;
|
||||
private pausedUntil = 0;
|
||||
private readonly lookups: Tally = { fallback: 'asking Nominatim', misses: 0, pausedUntil: 0 };
|
||||
private readonly suggestions: Tally = { fallback: 'suggesting from the index', misses: 0, pausedUntil: 0 };
|
||||
|
||||
constructor(@Inject(servicesConfig.KEY) private readonly config: ConfigType<typeof servicesConfig>) {}
|
||||
|
||||
/** Places matching the query, best first, or null when Nominatim has to answer. */
|
||||
search(query: string, opts: PhotonSearchOptions = {}): Promise<PhotonFeature[] | null> {
|
||||
const params = new URLSearchParams({ q: query, limit: String(SEARCH_LIMIT), lang: photonLang(opts.lang) });
|
||||
if (opts.bias && Number.isFinite(opts.bias.lat) && Number.isFinite(opts.bias.lng)) {
|
||||
params.set('lat', String(opts.bias.lat));
|
||||
params.set('lon', String(opts.bias.lng));
|
||||
params.set('zoom', String(BIAS_ZOOM));
|
||||
}
|
||||
return this.ask('api', params);
|
||||
return this.ask('api', searchParams(query, opts), this.lookups);
|
||||
}
|
||||
|
||||
/**
|
||||
* Places for the suggestion list, or null when it goes without them. A caller
|
||||
* that stops waiting aborts `signal`, and that counts as no miss.
|
||||
*/
|
||||
suggest(query: string, opts: PhotonSearchOptions & { signal?: AbortSignal } = {}): Promise<PhotonFeature[] | null> {
|
||||
if (Date.now() < this.lookups.pausedUntil) return Promise.resolve(null);
|
||||
return this.ask('api', searchParams(query, opts), this.suggestions, SUGGEST_TIMEOUT_MS, opts.signal);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -76,15 +95,18 @@ export class PhotonClient {
|
||||
reverse(lat: number, lng: number, lang?: string, timeoutMs?: number): Promise<PhotonFeature[] | null> {
|
||||
if (!Number.isFinite(lat) || !Number.isFinite(lng)) return Promise.resolve(null);
|
||||
const params = new URLSearchParams({ lat: String(lat), lon: String(lng), limit: '1', lang: photonLang(lang) });
|
||||
return this.ask('reverse', params, timeoutMs ? Math.min(TIMEOUT_MS, timeoutMs / 2) : TIMEOUT_MS);
|
||||
const deadline = timeoutMs ? Math.min(TIMEOUT_MS, timeoutMs / 2) : TIMEOUT_MS;
|
||||
return this.ask('reverse', params, this.lookups, deadline);
|
||||
}
|
||||
|
||||
private async ask(
|
||||
path: 'api' | 'reverse',
|
||||
params: URLSearchParams,
|
||||
tally: Tally,
|
||||
timeoutMs = TIMEOUT_MS,
|
||||
signal?: AbortSignal,
|
||||
): Promise<PhotonFeature[] | null> {
|
||||
if (!this.mayAsk()) return null;
|
||||
if (!this.mayAsk(tally)) return null;
|
||||
const url = new URL(`${this.config.photonUrl}/${path}`);
|
||||
url.search = params.toString();
|
||||
const identity = serviceIdentitySlot.get();
|
||||
@@ -95,13 +117,13 @@ export class PhotonClient {
|
||||
headers: { Accept: 'application/json', ...headers },
|
||||
// The key goes to this host alone, never on to wherever it points.
|
||||
redirect: 'error',
|
||||
signal: AbortSignal.timeout(timeoutMs),
|
||||
signal: signal ? AbortSignal.any([signal, AbortSignal.timeout(timeoutMs)]) : AbortSignal.timeout(timeoutMs),
|
||||
});
|
||||
identity.observe(url, res);
|
||||
const wait = res.status === 429 ? retryAfterMs(res.headers.get('retry-after')) : null;
|
||||
if (wait !== null) {
|
||||
await res.body?.cancel();
|
||||
this.pauseFor(wait);
|
||||
this.pauseFor(tally, wait);
|
||||
return null;
|
||||
}
|
||||
if (!res.ok) {
|
||||
@@ -109,37 +131,47 @@ export class PhotonClient {
|
||||
throw new Error(`answered ${res.status}`);
|
||||
}
|
||||
const { features } = photonAnswerSchema.parse(await capResponse(res, MAX_BYTES).json());
|
||||
this.misses = 0;
|
||||
tally.misses = 0;
|
||||
return features;
|
||||
} catch (err) {
|
||||
this.noteMiss(err);
|
||||
if (!signal?.aborted) this.noteMiss(tally, err);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/** Off with the services, beside an operator's own Nominatim, and during the pause. */
|
||||
private mayAsk(): boolean {
|
||||
private mayAsk(tally: Tally): boolean {
|
||||
if (this.config.mode === 'off' || readEnv().integrations.nominatimOwn) return false;
|
||||
return Date.now() >= this.pausedUntil;
|
||||
return Date.now() >= tally.pausedUntil;
|
||||
}
|
||||
|
||||
private noteMiss(err: unknown): void {
|
||||
private noteMiss(tally: Tally, err: unknown): void {
|
||||
const reason = err instanceof Error ? err.message : String(err);
|
||||
this.misses += 1;
|
||||
if (this.misses < MISSES_BEFORE_PAUSE) {
|
||||
logDebug(`[Photon] ${reason}; asking Nominatim instead`);
|
||||
tally.misses += 1;
|
||||
if (tally.misses < MISSES_BEFORE_PAUSE) {
|
||||
logDebug(`[Photon] ${reason}; ${tally.fallback} instead`);
|
||||
return;
|
||||
}
|
||||
this.pausedUntil = Date.now() + PAUSE_MS;
|
||||
tally.pausedUntil = Date.now() + PAUSE_MS;
|
||||
// One more miss after the pause starts the next one.
|
||||
this.misses = MISSES_BEFORE_PAUSE - 1;
|
||||
logWarn(`[Photon] ${reason}; asking Nominatim alone for a minute`);
|
||||
tally.misses = MISSES_BEFORE_PAUSE - 1;
|
||||
logWarn(`[Photon] ${reason}; ${tally.fallback} alone for a minute`);
|
||||
}
|
||||
|
||||
private pauseFor(ms: number): void {
|
||||
private pauseFor(tally: Tally, ms: number): void {
|
||||
const wait = Math.min(ms, MAX_PAUSE_MS);
|
||||
this.pausedUntil = Date.now() + wait;
|
||||
this.misses = MISSES_BEFORE_PAUSE - 1;
|
||||
logWarn(`[Photon] allowance spent; asking Nominatim alone for ${Math.ceil(wait / 1000)} s`);
|
||||
tally.pausedUntil = Date.now() + wait;
|
||||
tally.misses = MISSES_BEFORE_PAUSE - 1;
|
||||
logWarn(`[Photon] allowance spent; ${tally.fallback} alone for ${Math.ceil(wait / 1000)} s`);
|
||||
}
|
||||
}
|
||||
|
||||
function searchParams(query: string, opts: PhotonSearchOptions): URLSearchParams {
|
||||
const params = new URLSearchParams({ q: query, limit: String(SEARCH_LIMIT), lang: photonLang(opts.lang) });
|
||||
if (opts.bias && Number.isFinite(opts.bias.lat) && Number.isFinite(opts.bias.lng)) {
|
||||
params.set('lat', String(opts.bias.lat));
|
||||
params.set('lon', String(opts.bias.lng));
|
||||
params.set('zoom', String(BIAS_ZOOM));
|
||||
}
|
||||
return params;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* MAPS-AUTO-001..024: the suggestion list behind the place search box.
|
||||
* MAPS-AUTO-001..026: the suggestion list behind the place search box.
|
||||
*
|
||||
* This is the path the index was built for. Nominatim's usage policy names
|
||||
* autocomplete as unacceptable use in its own words, whatever the rate, and the
|
||||
@@ -11,6 +11,7 @@
|
||||
*/
|
||||
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
|
||||
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
|
||||
import { PHOTON_GRACE_MS } from '../../../src/nest/maps/maps-index.helpers';
|
||||
import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
|
||||
import { noGoogleQuota } from '../../helpers/google-quota';
|
||||
import { buildMapsParts, buildMapsService } from '../../helpers/maps-service';
|
||||
@@ -475,7 +476,7 @@ describe('MapsService.autocompletePlaces', () => {
|
||||
|
||||
const result = await svc.autocompletePlaces(1, INPUT, 'de', bias);
|
||||
|
||||
expect(photon).toHaveBeenCalledWith(INPUT, 'de', { lat: 54.1, lng: 12.1 });
|
||||
expect(photon).toHaveBeenCalledWith(INPUT, 'de', { lat: 54.1, lng: 12.1 }, expect.any(AbortSignal));
|
||||
expect(result.source).toBe('trek-places');
|
||||
expect(result.suggestions.map((s) => s.placeId)).toEqual([
|
||||
'gers:abc-123',
|
||||
@@ -516,4 +517,33 @@ describe('MapsService.autocompletePlaces', () => {
|
||||
expect(result.source).toBe('google');
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it('MAPS-AUTO-025: once the index has answered, a slow Photon gets a moment more and is then dropped', async () => {
|
||||
mockSearch.mockResolvedValue([hit()]);
|
||||
const { svc, photon } = withPhoton(null);
|
||||
photon.mockImplementation(() => new Promise(() => undefined));
|
||||
const started = Date.now();
|
||||
|
||||
const result = await svc.autocompletePlaces(1, INPUT);
|
||||
|
||||
expect(Date.now() - started).toBeGreaterThanOrEqual(PHOTON_GRACE_MS - 50);
|
||||
expect(result).toEqual({
|
||||
suggestions: [expect.objectContaining({ placeId: 'gers:abc-123' })],
|
||||
source: 'trek-places',
|
||||
});
|
||||
// The question nobody waits for any more is called off, which is no miss for Photon.
|
||||
expect((photon.mock.calls[0][3] as AbortSignal).aborted).toBe(true);
|
||||
});
|
||||
|
||||
it('MAPS-AUTO-026: with nothing from the index, Photon is waited for past that moment', async () => {
|
||||
mockSearch.mockResolvedValue([]);
|
||||
const { svc, photon } = withPhoton(null);
|
||||
photon.mockImplementation(
|
||||
() => new Promise((resolve) => setTimeout(() => resolve([photonRow()]), PHOTON_GRACE_MS + 100)),
|
||||
);
|
||||
|
||||
const result = await svc.autocompletePlaces(1, INPUT);
|
||||
|
||||
expect(result).toEqual({ suggestions: [photonRow()], source: 'openstreetmap' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/**
|
||||
* PHOTON-001..016: when Photon is asked, what it is asked, and every way it
|
||||
* PHOTON-001..019: when Photon is asked, what it is asked, and every way it
|
||||
* hands the question back to Nominatim. The network is a stubbed global fetch,
|
||||
* the installation identity a fake in the slot the service-identity module fills.
|
||||
*/
|
||||
@@ -197,3 +197,63 @@ describe('a spent allowance and a short deadline', () => {
|
||||
timeout.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('the suggestions asked per keystroke', () => {
|
||||
it('PHOTON-017: their misses and a spent allowance pause the suggestions alone, never the search', async () => {
|
||||
vi.useFakeTimers({ toFake: ['Date'] });
|
||||
vi.setSystemTime(new Date('2026-10-29T10:00:00Z'));
|
||||
const photon = client();
|
||||
fetchMock.mockImplementation(async () => photonAnswer([], 503));
|
||||
expect(await oneByOne([1, 2, 3, 4], () => photon.suggest('Ros'))).toEqual([null, null, null, null]);
|
||||
expect(await photon.suggest('Rost')).toBeNull();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(4);
|
||||
|
||||
fetchMock.mockImplementation(async () => photonAnswer([gateFeature()]));
|
||||
expect(await photon.search('Rostock')).toHaveLength(1);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(5);
|
||||
|
||||
vi.setSystemTime(new Date('2026-10-29T10:01:01Z'));
|
||||
fetchMock.mockResolvedValueOnce(
|
||||
new Response('{}', { status: 429, headers: { 'content-type': 'application/json', 'retry-after': '600' } }),
|
||||
);
|
||||
expect(await photon.suggest('Rost')).toBeNull();
|
||||
expect(await photon.suggest('Rosto')).toBeNull();
|
||||
expect(await photon.search('Rostock')).toHaveLength(1);
|
||||
expect(await photon.reverse(52.5, 13.4)).toHaveLength(1);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(8);
|
||||
});
|
||||
|
||||
it('PHOTON-018: a pause of the search holds them back too', async () => {
|
||||
vi.useFakeTimers({ toFake: ['Date'] });
|
||||
vi.setSystemTime(new Date('2026-10-29T10:00:00Z'));
|
||||
const photon = client();
|
||||
fetchMock.mockImplementation(async () => photonAnswer([], 503));
|
||||
await oneByOne([1, 2, 3, 4], () => photon.search('Rostock'));
|
||||
expect(await photon.suggest('Ros')).toBeNull();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
it('PHOTON-019: they wait less than a search, and one called off by its caller is no miss', async () => {
|
||||
const timeout = vi.spyOn(AbortSignal, 'timeout');
|
||||
const photon = client();
|
||||
expect(await photon.suggest('Ros', { lang: 'de', bias: { lat: 54.1, lng: 12.1 } })).toHaveLength(1);
|
||||
expect(timeout.mock.calls.map(([ms]) => ms)).toEqual([1500]);
|
||||
expect(request().url.searchParams.get('lat')).toBe('54.1');
|
||||
timeout.mockRestore();
|
||||
|
||||
fetchMock.mockImplementation(
|
||||
(_url: URL, init: RequestInit) =>
|
||||
new Promise((_resolve, reject) => {
|
||||
init.signal?.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError')));
|
||||
}),
|
||||
);
|
||||
for (let i = 0; i < 5; i += 1) {
|
||||
const dropped = new AbortController();
|
||||
const asked = photon.suggest('Rost', { signal: dropped.signal });
|
||||
dropped.abort();
|
||||
expect(await asked).toBeNull();
|
||||
}
|
||||
fetchMock.mockImplementation(async () => photonAnswer([gateFeature()]));
|
||||
expect(await photon.suggest('Rostock')).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user