99designs / 99designs/gqlgen

Feature request: @disableConcurrency directive to opt a field/object out of per-field goroutines

Đang mở
#4,321 2 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Go
Star
10.8k
Fork
1.3k
Merge trung bình
2 ngày 36 phút
Pull request đã merge (30 ngày)
26

Mô tả

## Summary

Add a schema directive (e.g. `@disableConcurrency`) that lets a field (or object) be resolved **inline on the parent goroutine** even when it is bound to a model method that takes `context.Context`.

Today the only way to keep a field off the goroutine path is to bind it to a plain struct field or a **context-free** method. That forces an unwanted choice: either take `ctx` (and pay for a goroutine you don't want) or drop `ctx` (and lose access to request-scoped
clients, loaders, metrics, and logging). A directive would decouple "the method needs `ctx`" from "this field should run concurrently."

## Current behavior

Whether a field is dispatched in its own goroutine is decided solely by `Field.IsConcurrent()` — `codegen/field.go:603`:

```go
func (f *Field) IsConcurrent() bool {
if f.Object.DisableConcurrency {
return false
}
return f.MethodHasContext || f.IsResolver
}
```

`DisableConcurrency` already exists on the `Object` — `codegen/object.go:34` — but it is only ever set for the **Mutation** root (to satisfy the spec's serial-execution requirement), and there is no way to set it from the schema or `gqlgen.yml` — `codegen/object.go:49`:

```go
DisableConcurrency: typ == b.Schema.Mutation,
```

`codegen/object.gotpl` then branches on `IsConcurrent`. Concurrent fields register into the `FieldSet` via `out.Concurrently(...)`; non-concurrent fields take the inline path:

```gotpl
{{- if $field.IsConcurrent }}
out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { ... })
{{- else }}
out.Values[i] = ec._{{$object.Name}}_{{$field.Name}}(ctx, field, obj)
{{- end }}
```

At runtime `FieldSet.Dispatch` runs the first registered field on the caller's goroutine and spawns a dedicated goroutine for **every other** registered field — `graphql/fieldset.go:37`:

```go
func (m *FieldSet) Dispatch(ctx context.Context) {
if len(m.delayed) == 1 {
// only one concurrent task, no goroutine
d := m.delayed[0]
m.Values[d.i] = d.f(ctx)
} else if len(m.delayed) > 1 {
var wg sync.WaitGroup
for _, d := range m.delayed[1:] { // every field except the first...
wg.Add(1)
go func(d delayedResult) { // ...gets its own goroutine
defer wg.Done()
m.Values[d.i] = d.f(ctx)
}(d)
}
m.Values[m.delayed[0].i] = m.delayed[0].f(ctx)
wg.Wait()
}
}
```

So a selection set of N concurrent fields spawns N−1 goroutines — regardless of whether those fields do any real I/O.

## The `ctx` cliff (the core problem)

Consider a field bound to a model method. The presence of `context.Context` in the signature is the *only* difference, yet it flips concurrency on:

```go
// (A) Runs in a DEDICATED GOROUTINE.
// MethodHasContext == true -> IsConcurrent() == true.
func (m *Model) Field1(ctx context.Context) (string, error) {
// needs ctx: request-scoped clients, dataloaders, metrics, logging...
return compute(ctx)
}
```

```go
// (B) Runs INLINE on the parent goroutine.
// MethodHasContext == false -> IsConcurrent() == false.
func (m *Model) Field1() (string, error) {
// no ctx available — cannot reach request-scoped clients/loaders.
return m.precomputed, nil
}
```

For a field whose work is trivial (e.g. reading an already-memoized value) but which still needs `ctx` to reach request-scoped dependencies, neither option is good:

- **(A)** pays for a goroutine to do near-zero work.
- **(B)** avoids the goroutine but can't use `ctx`, so the value must be precomputed eagerly upstream — which over-fetches when the field isn't selected. (Stashing `ctx` on the struct to fake a context-free method is an anti-pattern.)

On large lists this is real overhead: M assets × K such fields ≈ M·(K−…) goroutines doing trivial cached reads, adding scheduler pressure for no parallelism benefit.

## Proposed solution

A directive that sets `DisableConcurrency` for the annotated field (and/or object), independent of `MethodHasContext`:

```graphql
directive @disableConcurrency on FIELD_DEFINITION | OBJECT

type Model {
# keeps the ctx-taking method, but resolved inline on the parent goroutine
field1: String! @disableConcurrency
}
```

With `@disableConcurrency`, example (A) above would generate the inline path (`out.Values[i] = ec._Model_field1(ctx, field, obj)`) instead of `out.Concurrently(...)`, even though the method takes `ctx`.

### Suggested implementation sketch

- Recognize the directive during object/field build and set a per-field disable flag (or
reuse/propagate `Object.DisableConcurrency` at field granularity).
- Extend `Field.IsConcurrent()`:

```go
func (f *Field) IsConcurrent() bool {
if f.Object.DisableConcurrency || f.DisableConcurrency {
return false
}
return f.MethodHasContext || f.IsResolver
}
```

- No runtime change needed — the existing inline template branch and `FieldSet` already handle non-concurrent fields; the directive only changes which branch codegen emits.

### Scope note for resolver fields

For plain model-method fields this is straightforward. If the directive is also allowed on `@goField(forceResolver: true)` fields, the generated resolver call would run inline in the selection loop rather than via `Dispatch`. That's a reasonable extension but could be
deferred; the model-method case alone already covers the main use case.

## Alternatives considered

- **Context-free method / plain struct field.** Works, but forces eager precomputation and loses `ctx` access (see above).
- **`worker_limit` in `gqlgen.yml`.** Global, and only caps concurrency for list-element marshaling (`MarshalSliceConcurrently`, `codegen/type.gotpl:138`); it does not make individual object fields inline.
- **Expose the existing `Object.DisableConcurrency` via `@goModel` / config.**
Object-level only — too coarse when some fields on the object genuinely benefit from parallel I/O and only a few (cheap, cached) fields want to opt out. A field-level directive is the finer-grained fit.

## Backward compatibility

Fully additive. The directive defaults to absent, so existing schemas generate identical code. Only annotated fields change from the concurrent path to the inline path.

## Use case

Fields that resolve from a per-request memoized/loader-backed cache: the expensive fetch is already de-duplicated, so the remaining per-field work is a trivial read, and spawning a goroutine per field is pure overhead — but the method still needs `ctx` to reach the cache/loader. `@disableConcurrency` would let such fields keep `ctx` and resolve inline.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Bắt đầu bằng cách đọc các tệp codegen/field.go và codegen/object.go để hiểu IsConcurrent() và DisableConcurrency hoạt động như thế nào. Directive cần được nhận diện trong quá trình phân tích schema và thiết lập một cờ cho từng field. Chỉnh sửa template trong codegen/object.gotpl để sử dụng đường dẫn inline cho các field có directive. Kiểm thử bằng cách thêm một directive vào một field trong schema kiểm thử và xác minh rằng mã được sinh ra không sử dụng out.Concurrently.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Lĩnh vực
api, backend
Loại issue
Tính năng
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
65/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.