CloudFetch and M2M token exchange are unbounded and uninterruptible (v1.13.0)
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Idoneità per principianti
- 35/100
- Tipo di issue
- Bug
- Chiarezza
- Abbastanza chiara
- Stato di attività
- Attiva
- Ambito
- authentication, backend, backend-api-design, performance
Direzione di ricerca
Inizia tracciando il percorso di download in internal/rows/rows.go, internal/rows/arrowbased/arrowRows.go e batchloader.go, quindi esamina auth/oauth/m2m/m2m.go e le relative voci di connector e config. Usa gli entry point esistenti di CloudFetch e dell’autenticazione per comprendere il comportamento del contesto, del client, della scadenza e del token. Il lavoro è completato quando i quattro percorsi segnalati hanno un comportamento delimitato e interrompibile oppure una condizione terminale documentata esplicitamente, e il commento di WithCloudFetch corrisponde al valore predefinito.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Version
v1.13.0
Summary
CloudFetch result downloads are unbounded and uninterruptible, and an expired
presigned link ends the read with no recovery path. Four issues, the first three
in the download path and the fourth on the auth path.
1. The caller's context is discarded
internal/rows/rows.go:622 and :624 pass r.ctx into NewArrowRowScanner,
but internal/rows/arrowbased/arrowRows.go:124 substitutes context.Background()
when constructing the iterator:
bi, err2 = NewCloudBatchIterator(context.Background(), rowSet.ResultLinks, ...)
So the context given to QueryContext never reaches the download. Cancelling the
query, or its deadline expiring, cannot stop an in-progress file fetch.
2. The download client has no timeout and no public setter
internal/rows/arrowbased/batchloader.go:62 falls back to http.DefaultClient,
which has no Timeout:
httpClient := http.DefaultClient
if cfg.HTTPClient != nil {
httpClient = cfg.HTTPClient
}
CloudFetchConfig.HTTPClient has no ConnOption. The only way to populate it is
WithTransport, which also replaces the Thrift transport as a side effect, and
the http.Client it builds (connector.go:421) sets no Timeout either.
Related: #307 asked for a configurable CloudFetch transport and was closed as
completed. WithTransport addresses the transport case, but a RoundTripper
cannot set http.Client.Timeout, so the timeout case above is still open.
Together with (1), a stalled download has neither a timeout nor working
cancellation. Because downloads are lazy (cloudIPCStreamIterator.Next(),
batchloader.go:200), the caller is blocked inside row iteration, so a deferred
rows.Close() cannot fire to break the stall.
3. Expired links are terminal
batchloader.go:453 returns errors.New(dbsqlerr.ErrLinkExpired). That constant
appears nowhere else in the module apart from its own definition: nothing handles
it and nothing re-issues links. A result set large enough that its presigned URLs
expire part-way through reading cannot be completed, only restarted.
CloudFetchConfig.MinTimeToExpiry defaults to 0 (internal/config/config.go:557
clamps only negative values) and likewise has no ConnOption, so a caller cannot
widen the safety margin.
4. The token exchange is unbounded and serialized
auth/oauth/m2m/m2m.go:41 holds authClient.mx across the token fetch, and the
token source is built with context.Background() (m2m.go:73), so
clientcredentials falls back to http.DefaultClient with no timeout. A hung
token endpoint therefore stalls every in-flight request on that connector
indefinitely, with no way for the caller to bound it. Endpoint discovery is
capped at 10s (auth/oauth/oauth.go:21), but the exchange itself is not.
Also, a doc mismatch
WithCloudFetch's comment (connector.go:429) says "Default is false", but
CloudFetchConfig.WithDefaults() sets UseCloudFetch = true unconditionally
(internal/config/config.go:546). CloudFetch is on by default.
Suggested fixes
- Pass the scanner's context through to
NewCloudBatchIteratorinstead of
context.Background(). - Expose
HTTPClientandMinTimeToExpiryasConnOptions, so a caller can bound
downloads without displacing the Thrift transport. Note thatWithTransportis
not a substitute: aRoundTrippercannot sethttp.Client.Timeout, so it can
only bound each hop of a redirect chain rather than the request as a whole. - Bound the token exchange, and consider not holding the mutex across it.
- Re-fetch result links on expiry, or if that is out of scope, document the
condition as terminal so callers can plan for it.
- Lingua principale
- Go
- Stelle
- 53
- Fork
- 66
- Merge medio
- 7h 57m
- PR unite (30g)
- 16
Guida per i contributori
Apri la guida per i contributori
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 databricks/databricks-sql-go
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
databricks/databricks-sql-go#476 ·
-
Difficoltà 3/5 1-2 giorni Idoneità per principianti 55/100
databricks/databricks-sql-go#481 ·
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 52/100
databricks/databricks-sql-go#480 ·
-
Difficoltà 3/5 1-2 giorni Idoneità per principianti 65/100
databricks/databricks-sql-go#474 ·
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 62/100
databricks/databricks-sql-go#473 ·
Tutte le issue di databricks/databricks-sql-go
Issue simili
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 60/100
github/gh-aw-mcpg#13748 ·
-
agentic-workflows
Difficoltà 2/5 1-3 ore Idoneità per principianti 65/100
-
needs-triage
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 65/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
googleapis/librarian#7670 · 2 commenti ·