dgraph-io / dgraph-io/dgraph4j

Make runWithRetries non-blocking so ingest concurrency isn't capped by the executor thread count

Open
#292 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

kind/enhancement Stale
Dominant language
Java
Stars
170
Forks
62
PR merge metrics
No merged PRs in 30d

Description

What

CompletableFutures.runWithRetries wraps every query/mutation/commit as CompletableFuture.supplyAsync(() -> ctxCallable.call().get(), executor). The .get() blocks the executor thread for the entire gRPC round-trip, even though the underlying stub call is already async — the response arrives via StreamObserverBridge on a gRPC event-loop thread. So each in-flight request holds one executor thread until it completes.

The default executor is ForkJoinPool.commonPool() (parallelism ≈ availableProcessors() - 1). Net effect: concurrent in-flight transactions are hard-capped at roughly the client's core count, regardless of how many transactions the caller launches — extra work just queues in the pool.

Why it matters

For high-throughput ingest against a multi-core alpha this starves the server: a client on an N-core box keeps at most ~N requests in flight, so a large alpha sees only ~N concurrent mutations and its apply cores sit idle. Turning up ingest parallelism on the client has no effect past the pool size. Today's workarounds (pass a large custom Executor, use multiple channels) mitigate it but burn a blocked thread per in-flight request and require every caller to know the trap — and the ergonomic DgraphClient.ClientOptions.build() path (single channel + default commonPool) is the least concurrent of all.

Proposed fix

Rewrite runWithRetries to compose on the future instead of blocking on it: return the CompletableFuture from ctxCallable.call() directly and attach the JWT-expiry retry via .handle/.thenCompose rather than supplyAsync(() -> ...get()). No thread is held for the round-trip; in-flight concurrency is then bounded only by HTTP/2 streams (and whatever the caller chooses), not by the executor's size.

Preserve: the single JWT-refresh-then-retry-once semantics, gRPC Context propagation (Context.current().wrap(...)), and exception translation.

Acceptance

  • No executor thread is blocked for the duration of a request's round-trip.
  • Concurrent in-flight request count is not bounded by the executor's parallelism (verify: launch M ≫ pool-size async mutations, observe ~M in flight).
  • Existing retry / JWT-refresh and exception-translation behavior unchanged; tests green.

Refs

  • src/main/java/io/dgraph/CompletableFutures.javarunWithRetries
  • src/main/java/io/dgraph/DgraphAsyncClient.java — executor defaults to ForkJoinPool.commonPool()
  • src/main/java/io/dgraph/AsyncTransaction.javadoRequest/commit route through runWithRetries

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/main/java/io/dgraph/CompletableFutures.java at runWithRetries, then trace its callers in DgraphAsyncClient.java and AsyncTransaction.java. Review how Context.current().wrap(...), JWT-refresh retry behavior, and exception translation are currently handled. Done means round trips no longer occupy executor threads while existing retry behavior and tests remain unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
grpc, java
Domain
api, performance
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.