[Bug][Collector] Stateful collector silently discards the final page when ResponseParser returns items with ErrFinishCollect
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Idoneità per principianti
- 65/100
- Tipo di issue
- Bug
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Attiva
- Stack tecnologico
- go
- Ambito
- backend, data-engineering
Direzione di ricerca
The bug is in backend/helpers/pluginhelper/api/api_collector_stateful.go. Start by reading the ResponseParser wrapper and the fetchAsync method. The fix is to modify the wrapper to return items when err is ErrFinishCollect. Test with the CircleCI plugin's pipeline_collector.go, ensuring the last page is not discarded. Run the reproduction steps to verify the fix.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Description
api_collector_stateful.go's ResponseParser wrapper discards the records returned by the
plugin's own parser whenever that parser signals ErrFinishCollect. The result is a silent,
page-aligned data loss at the timeAfter boundary: the last page of a time-bounded collection
is dropped in full, including the records on it that are inside the requested window.
Nothing errors. The task reports TASK_COMPLETED with zero failed subtasks, and every
internal consistency check passes, because the tool tables and the domain tables agree with
each other — they are both derived from the same truncated raw data. The loss is only visible
by comparing against the upstream API directly.
Root cause
backend/helpers/pluginhelper/api/api_collector_stateful.go:
ResponseParser: func(res *http.Response) ([]json.RawMessage, errors.Error) {
items, err := args.CollectNewRecordsByList.ResponseParser(res)
if err != nil {
return nil, err // <-- discards `items` even when err is ErrFinishCollect
}
Plugin parsers are written to return the valid prefix of a page together with the stop signal.
For example backend/plugins/circleci/tasks/pipeline_collector.go:
for _, item := range data.Items {
pipelineCreatedAt, err := extractCreatedAt(item)
if err != nil { return nil, err }
if pipelineCreatedAt.Before(*timeAfter) {
return filteredItems, api.ErrFinishCollect // in-window items + stop
}
filteredItems = append(filteredItems, item)
}
The wrapper replaces filteredItems with nil. ApiCollector.fetchAsync then takes the
count == 0 early return and never reaches db.Create, so the page is never persisted:
items, err := collector.args.ResponseParser(res)
if err != nil {
if errors.Is(err, ErrFinishCollect) {
logger.Info("a fetch stop by parser, reqInput: #%s", reqData.Params)
handler = nil
} ...
}
count := len(items)
if count == 0 {
collector.args.Ctx.IncProgress(1)
return nil // <-- page silently dropped
}
Note fetchAsync itself handles this correctly — it persists whatever it is given before
stopping pagination. The loss is introduced purely by the wrapper nulling the slice.
This affects any plugin using NewStatefulApiCollectorForFinalizableEntity whose
ResponseParser returns a partial page with ErrFinishCollect, not only circleci.
Regression lineage
The circleci parser gained its return filteredItems, api.ErrFinishCollect line in #7820
(merged 2024-08-15), which fixed #7797 "CircleCI pipelines collected from before time range".
That plugin-side change is correct on its own terms — it stops collection at the window
boundary instead of running past it. But because the stateful wrapper nulls the slice whenever
the parser returns an error, the fix effectively traded collecting too much for silently
collecting too little. #7797's symptom was visible in the data; this one is not.
Impact
Loss is bounded by one page, so it is proportionally worst for low-volume scopes — the
opposite of where it is likely to be noticed. Measured on three repositories with
timeAfter = 90 days, PageSize = 20 (the CircleCI default):
| repository | upstream API | collected | lost | loss |
|---|---|---|---|---|
| A (0.38 pipelines/day) | 34 | 20 | 14 | 41% |
| B (3.1 pipelines/day) | 276 | 260 | 16 | 5.8% |
| C (6.3 pipelines/day) | 568 | 560* | 8 | 1.4% |
* predicted from the same model; C was collected after the workaround was applied and
returned the full 568.
Reproduction
- Configure a CircleCI connection and a project scope for a repository with more pipelines in
the window than one page (>20), where the window boundary falls mid-page. - Set a blueprint
timeAftersuch that the boundary page contains both in-window and
out-of-window pipelines. - Run the blueprint. It completes
TASK_COMPLETED, zero failed subtasks. SELECT count(*) FROM _tool_circleci_pipelines WHERE project_slug = '...'returns an exact
multiple of the page size.- Compare with
GET /v2/project/{slug}/pipelinecounted directly over the same window — the
difference is the in-window records on the discarded page.
Confirming signals, all consistent:
_raw_circleci_api_pipelinesholds the same truncated count, so the loss is at collection,
not extraction.- Collected pipeline
numbervalues are perfectly contiguous — the tail is amputated, records
are not dropped at random. - A
fullSync: truere-run reproduces the identical count. - In-band detector:
collectPipelines.finishedRecordscounts pages fetched while
extractPipelines.finishedRecordscounts records kept;pages - kept/PageSizeis the number
of discarded pages. This agreed with thea fetch stop by parserlog line on exactly the runs
that lost data.
Suggested fix
Preserve the partial page when the stop signal is ErrFinishCollect:
items, err := args.CollectNewRecordsByList.ResponseParser(res)
if err != nil {
if errors.Is(err, ErrFinishCollect) {
return items, err
}
return nil, err
}
fetchAsync already persists the returned items and then stops paginating, so no other change
is needed.
Workaround for operators
Set timeAfter earlier than the window you actually need, so the discarded page falls in the
buffer rather than in the data you rely on. The buffer must exceed one page expressed in
time, which varies with each scope's rate: at PageSize 20 that is ~3 days for a repository
at 6.3 pipelines/day but ~53 days for one at 0.38/day. A fixed buffer chosen for busy scopes
will not protect quiet ones.
Environment
- DevLake
v1.0.3-beta17(8fe26f4), Helm chart1.0.3-beta9 - PostgreSQL backend
- Plugins: circleci, github_graphql
PIPELINE_MAX_PARALLEL=1,API_REQUESTS_PER_HOUR=3000
- Lingua principale
- Go
- Stelle
- 3.1k
- Fork
- 812
- Merge medio
- 2g 9h
- PR unite (30g)
- 56
Guida per i contributori
Nessuna guida per i contributori indicizzata per questo repository
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di apache/devlake
-
type/bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 82/100
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
-
type/bug
Difficoltà 4/5 3-5 giorni Idoneità per principianti 52/100
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
-
apache/devlake#9142 · 2 commenti · 1 reazione · 1 assegnatario ·
Tutte le issue di apache/devlake
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
bazel-contrib/rules_go#4726 · 1 commento ·
-
area/auto-scaling area/monitoring area/ops-productivity kind/enhancement
Difficoltà 2/5 1-3 ore Idoneità per principianti 74/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
-
Type/Improvement
Difficoltà 1/5 1-3 ore Idoneità per principianti 90/100
OpenNSW/nsw-srilanka#522 ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 92/100