Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions CACHING.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,30 @@ A warm that finishes without a complete result puts the term on a cooldown
(`VFBQUERY_PREVIEW_WARM_COOLDOWN`, default 300s) so a term whose previews cannot
be computed does not queue one warm per request and starve the terms that can.

**Fixed in v1.22.37:** the deferral above is only safe while it is *temporary*,
and it had stopped being temporary. `cache_result` wrote its documents with
`commit=false`, deferring visibility to the core's `autoSoftCommit` — but the
cache core is configured `autoSoftCommit.maxTime: -1` and
`autoCommit.openSearcher: false`, so no searcher ever reopens on its own. Writes
were durable and returned HTTP 200, yet nothing written could ever be read back.
Every request was therefore a cold miss, every cold miss took the fast path, and
every response carried `count: -1` — permanently, for every term. Writes now use
`commitWithin` (which this core *does* honour, with `softCommit: true`), so they
stay non-blocking but become searchable within ~10s
(`VFBQUERY_SOLR_COMMIT_WITHIN_MS`); `VFBQUERY_SOLR_WRITE_COMMIT=true` still
forces the old blocking hard commit. The same `commit=false` bug silently
disabled the expired-document delete, and is fixed alongside it.

Two related changes went in with it. `preview_results` now carries an optional
`status` (`pending`/`complete`) and a `message` explaining what an unresolved
preview means and how to resolve it, because `count: -1` reads as "no results"
to anyone who has not read this page; absence of `status` keeps meaning
complete, since the cache holds three months of entries written without it. And
the read/write validators now ask `preview_is_resolved()` rather than testing
`count >= 0` directly, which incidentally fixes a term whose preview is complete
but whose exact total exceeded `COUNT_CAP`: its `count` is `-1` meaning "many",
and such a term was being rejected from cache and recomputed on every request.

### Deliberately not cached

- `get_similar_morphology_userdata` — keyed on a per-session user upload id;
Expand Down
23 changes: 23 additions & 0 deletions docs/http-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,29 @@ GET /get_term_info?id=FBbt_00007401
| `id` | **Required.** A VFB short_form: `FBbt_…` (anatomy class), `VFB_…` (individual), `VFBexp_…`, `FBgn_…`, `FBlc_…`. |
| `force_refresh` | `true` bypasses the result cache for this call. |

### Query previews may be pending

Each entry in `Queries` carries a preview of that query's results. Computing all of them takes tens of
seconds on a cold term, so the first request for a term returns without them and warms them in the
background. Such a preview has empty `rows` and a **`count` of `-1`, which means *not counted* — not
zero.** It says nothing about whether results exist.

`preview_results.status` names the state (`pending` or `complete`) and `preview_results.message`
explains it in a sentence. Both are optional and their **absence means complete**, because entries
cached before they existed carry neither; the fallback rule is `count >= 0`. A `complete` preview can
also carry `count: -1`, in the one case where `-1` means "many": the rows are final, but the exact
total exceeded the counting cap. Ask again shortly and a pending preview is usually filled in.

### `X-Force-Refresh`

`/get_term_info`, `/run_query` and `/query_connectivity` accept `X-Force-Refresh: true|1|yes|on` as a
header spelling of `force_refresh=true`. It exists because the `v3-cached` layer in front of this
service already defines that header as "bypass the edge cache and overwrite the cached entry with the
fresh upstream response", and reserves it for whitelisted callers. Sending the header refreshes both
layers in one request, at the URL users actually call. Adding `&force_refresh=true` instead does not:
the edge cache is keyed on the request URI, so the refreshed response lands in a *different* cache
entry and the one users hit is never healed.

## `/run_query`

```
Expand Down
35 changes: 34 additions & 1 deletion schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ This document describes the JSON schema structure for the Virtual Fly Brain (VFB
## Table of Contents

- [Core Schema](#core-schema)
- [Query previews and the -1 count](#query-previews-and-the--1-count)
- [Entity Types](#entity-types)
- [Individual](#individual)
- [Class](#class)
Expand Down Expand Up @@ -51,6 +52,8 @@ The base schema returned by term info queries:
"preview": "Integer (number of preview results, -1 for all)",
"preview_columns": ["String (column identifiers)"],
"preview_results": {
"status": "String (pending|complete; optional — absent means complete)",
"message": "String (present with status; explains a pending or uncounted preview)",
"headers": {
"column_id": {
"title": "String (display name)",
Expand All @@ -68,7 +71,7 @@ The base schema returned by term info queries:
]
},
"output_format": "String (table/ribbon)",
"count": "Integer (total result count)"
"count": "Integer (total result count; -1 = not counted, distinct from 0)"
}
],
"IsIndividual": "Boolean",
Expand All @@ -85,6 +88,34 @@ The base schema returned by term info queries:
}
```

### Query previews and the -1 count

Each entry in `Queries` describes a query the caller can run against the term, and carries a small
preview of that query's results so a client can show something useful without running anything. A
preview is expensive, so it is not always available when the term itself is, and the schema has to be
able to say so.

`count` is that signal. A count of `0` means the query was run and matched nothing. A count of `-1`
means the query has **not been counted**, which is a different statement entirely: it says nothing
about whether results exist, only that finding out requires running the query. Treating `-1` as "no
results" is the single most common misreading of this schema. (Note the unrelated `-1` on the
`preview` field just above it, which means "preview every result" — the two are not related.)

A preview can be uncounted for four reasons. It has not been computed yet, because this was the first
request for the term and the full previews are being computed in the background; it timed out inside
its share of the response budget; it failed; or — the one case where `-1` does *not* mean unknown —
the rows are complete but the exact total exceeded the counting cap, so `-1` here means "many".

`preview_results.status` distinguishes those. It is `pending` when the rows are not the answer and
`complete` when they are, and `preview_results.message` accompanies it with a sentence saying which
case this is and what to do about it. Both keys are optional, and **absence means complete**: results
cached before these keys existed carry neither, and remain valid. So the rule for a consumer is:
trust `status` when it is present, and fall back to `count >= 0` when it is not.

A `pending` preview is transient, not an error. Requesting the same term again shortly will usually
return it filled in, since the first request schedules the computation; passing `force_refresh=true`
(or the `X-Force-Refresh: true` header, from a whitelisted caller) computes it synchronously instead.

## Entity Types

VFB entities fall into three main types, each with specific fields beyond the core schema:
Expand Down Expand Up @@ -422,6 +453,7 @@ Finds individuals related to a template.
"preview": 5,
"preview_columns": ["id", "score", "name", "tags", "thumbnail"],
"preview_results": {
"status": "complete",
"headers": {
"id": {"title": "Add", "type": "selection_id", "order": -1},
"score": {"title": "Score", "type": "numeric", "order": 1, "sort": {"0": "Desc"}},
Expand Down Expand Up @@ -554,6 +586,7 @@ Finds individuals related to a template.
"preview": 5,
"preview_columns": ["id", "name", "driver", "thumbnail"],
"preview_results": {
"status": "complete",
"headers": {
"id": {"title": "Add", "type": "selection_id", "order": -1},
"name": {"title": "Name", "type": "markdown", "order": 1, "sort": {"0": "Asc"}},
Expand Down
2 changes: 1 addition & 1 deletion src/vfbquery/_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@
# Targeted invalidation, not a namespace flip. This is the documented exception,
# not a licence to hand-manage the cache in general — the standing rule remains
# force_refresh per call or a major.minor bump, never renaming buckets.
__version__ = "1.22.36"
__version__ = "1.22.37"
19 changes: 9 additions & 10 deletions src/vfbquery/cached_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"""

from typing import Dict, Any, Optional
from .solr_result_cache import with_solr_cache
from .solr_result_cache import with_solr_cache, preview_is_resolved


def is_valid_term_info_result(result):
Expand All @@ -21,24 +21,23 @@ def is_valid_term_info_result(result):
# Additional validation for query results
if 'Queries' in result:
for query in result['Queries']:
# Check if query has invalid count (-1) which indicates failed execution
# Note: count=0 is valid if preview_results structure is correct
count = query.get('count', 0)

# Check if preview_results has the correct structure
preview_results = query.get('preview_results')
if not isinstance(preview_results, dict):
# print(f"DEBUG: Invalid preview_results type {type(preview_results)} detected")
return False

headers = preview_results.get('headers', [])
if not headers:
# print(f"DEBUG: Empty headers detected in preview_results")
return False

# Only reject if count is -1 (failed execution) or if count is 0 but preview_results is missing/empty
if count < 0:
# print(f"DEBUG: Invalid query count {count} detected")

# Reject a preview that never resolved: count=0 is a valid answer
# ("no matches"), count=-1 is "not counted yet" and is not. A
# preview explicitly marked complete stays valid even at count -1,
# where -1 means "more than the counting cap" rather than "unknown".
if not preview_is_resolved(query):
# print(f"DEBUG: Unresolved query preview detected")
return False

return True
Expand Down
56 changes: 50 additions & 6 deletions src/vfbquery/ha_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -930,12 +930,13 @@ async def handle_get_term_info(request):
{"error": "Missing required parameter: id"}, status=400
)

force_refresh = _query_flag(request, "force_refresh")
force_refresh = _force_refresh_requested(request)

warnings = _unknown_param_warnings(request, _TERM_INFO_PARAMS)
warn = _flag_warning(request, "force_refresh")
if warn:
warnings.append(warn)
for warn in (_flag_warning(request, "force_refresh"),
_force_refresh_header_warning(request)):
if warn:
warnings.append(warn)

def finish(result):
return web.json_response(_with_warnings(result, warnings))
Expand Down Expand Up @@ -1114,6 +1115,43 @@ def _query_flag(request, name, default=False):
return str(raw).strip().lower() in _TRUE_VALUES


#: Header spelling of ``force_refresh``. The v3-cached nginx layer in front of
#: this service already defines ``X-Force-Refresh: true|1|yes|on`` (from a
#: whitelisted IP) as "bypass the edge cache and overwrite the canonical slot
#: with a fresh upstream response". Until this service honoured it too, that
#: header refreshed the edge from an *unrefreshed* upstream — and the obvious
#: workaround, appending ``&force_refresh=true``, changes ``$request_uri`` and
#: therefore the nginx cache key, so it writes a different slot and can never
#: heal the canonical one. Accepting the header here closes that seam: one
#: request refreshes both layers, at the URL users actually call.
_FORCE_REFRESH_HEADER = "X-Force-Refresh"


def _force_refresh_requested(request):
"""True when this request asks for a refresh, by query param or header.

Either spelling is sufficient; neither overrides the other.
"""
if _query_flag(request, "force_refresh"):
return True
raw = request.headers.get(_FORCE_REFRESH_HEADER)
if raw is None:
return False
return str(raw).strip().lower() in _TRUE_VALUES


def _force_refresh_header_warning(request):
"""Warn about an ``X-Force-Refresh`` value that is neither a yes nor a no."""
raw = request.headers.get(_FORCE_REFRESH_HEADER)
if raw is None:
return None
value = str(raw).strip().lower()
if value in _TRUE_VALUES or value in _FALSE_VALUES:
return None
return ("%s: %r is not a recognised boolean and was read as false; use %s"
% (_FORCE_REFRESH_HEADER, str(raw), " / ".join(_TRUE_VALUES)))


#: Spellings of "no" a caller might reasonably write for a flag. Everything
#: outside these two sets is neither yes nor no — it is a typo, and
#: :func:`_flag_warning` says so instead of quietly meaning "no".
Expand Down Expand Up @@ -1338,13 +1376,16 @@ async def handle_run_query(request):
)

include_graph = _query_flag(request, "include_graph")
force_refresh = _query_flag(request, "force_refresh")
force_refresh = _force_refresh_requested(request)

warnings = _unknown_param_warnings(request, _RUN_QUERY_PARAMS)
for flag in ("include_graph", "force_refresh"):
warn = _flag_warning(request, flag)
if warn:
warnings.append(warn)
warn = _force_refresh_header_warning(request)
if warn:
warnings.append(warn)

# `include_graph` is honoured by four of the forty query types. Asking for
# it on any of the other thirty-six used to return a graphless result that
Expand Down Expand Up @@ -1992,7 +2033,7 @@ async def handle_query_connectivity(request):
from .vfb_connectivity import DEFAULT_EXCLUDE_DBS
exclude_dbs = list(DEFAULT_EXCLUDE_DBS)
include_graph = _query_flag(request, "include_graph")
force_refresh = _query_flag(request, "force_refresh")
force_refresh = _force_refresh_requested(request)

# Resolved before the cache key is built, so `exclude_dbs=male-cns` and
# `exclude_dbs=mc` share one entry instead of computing the same answer
Expand All @@ -2013,6 +2054,9 @@ async def handle_query_connectivity(request):
warn = _flag_warning(request, flag)
if warn:
warnings.append(warn)
warn = _force_refresh_header_warning(request)
if warn:
warnings.append(warn)

def post_fn(result):
if not isinstance(result, dict):
Expand Down
Loading
Loading