JanusGraph / JanusGraph/janusgraph

Elasticsearch: transient failures classified as PermanentBackendException, so index mutations are silently dropped instead of retried

Open
#4,925 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
5.8k
Forks
1.2k
Avg merge
13h 53m
Merged PRs (30d)
6

Description

- Version: `master` (`ac0eb23`) — also all released versions; `convert()` has had this shape since the Titan era
- Storage Backend: any (behaviour is not backend-specific)
- Mixed Index Backend: elasticsearch
- Expected Behavior: transient Elasticsearch failures (HTTP 429/502/503/504, connection reset, socket timeout) should be classified as `TemporaryBackendException`, so that the retry loop already present in `BackendOperation` absorbs them within the `storage.write-time` budget.
- Current Behavior: every exception except `InterruptedException` is classified as `PermanentBackendException`, so transient failures are never retried. The index mutation is dropped, and because index mutations are applied *after* storage in `StandardJanusGraph.commit()`, the failure cannot be rolled back — it is only logged, leaving the mixed index permanently inconsistent with the graph.

### Details

`ElasticSearchIndex.convert()` collapses every failure mode into one bucket:

https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java#L543-L549

```java
private BackendException convert(Exception esException) {
if (esException instanceof InterruptedException) {
return new TemporaryBackendException("Interrupted while waiting for response", esException);
} else {
return new PermanentBackendException("Unknown exception while executing index operation", esException);
}
}
```

The retry machinery already exists and is already wired up, so this is a classification fix rather than new functionality. `IndexTransaction.flushInternal()` runs the mutation through `BackendOperation.execute(..., maxWriteTime)`:

https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-core/src/main/java/org/janusgraph/diskstorage/indexing/IndexTransaction.java#L141-L159

and `BackendOperation.executeDirect` implements jittered exponential backoff — but only when the innermost `BackendException` is a `TemporaryBackendException`:

https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-core/src/main/java/org/janusgraph/diskstorage/util/BackendOperation.java#L59-L98

So today:

- ES returns 429 `es_rejected_execution_exception` (write threadpool queue full) → `PermanentBackendException` → no retry → mutation dropped.
- ES returns 502/503/504 (shard unavailable, gateway restart, rolling upgrade) → same.
- Socket timeout or connection reset mid-bulk → same.

Because `commit()` commits storage first and then collects index failures rather than aborting:

https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/StandardJanusGraph.java#L1047-L1054

the dropped mutation cannot be undone. Unless `log-tx` is enabled (default `false`) *and* a transaction-recovery processor is running, the divergence is permanent and the only signal is a single ERROR log line.

The net effect is that a brief and entirely routine Elasticsearch event — a rolling restart, a hot shard rejecting a bulk, a GC pause causing a socket timeout — permanently desynchronizes a mixed index, in a situation the existing retry loop was designed to absorb.

### Steps to Reproduce

1. Configure a graph with an Elasticsearch mixed index.
2. Make ES return a retryable error for a bulk request — e.g. saturate the write threadpool queue so bulk items fail with `es_rejected_execution_exception`, or interpose a proxy that returns 503 for one request.
3. Commit a transaction that writes to that mixed index.
4. Observe: `Error while committing index mutations for transaction [...]` is logged once, no retry is attempted, and the document is absent from ES while the element is present in the storage backend.

### Suggested Fix

Classify on transport / HTTP status rather than a single `instanceof`:

- `TemporaryBackendException`: HTTP 429, 502, 503, 504; `ConnectException`, `SocketTimeoutException`, `ConnectionClosedException`, `NoHttpResponseException`; `InterruptedException` (unchanged)
- `PermanentBackendException`: HTTP 400 (including `mapper_parsing_exception`), 401/403, 404, and anything unrecognised

The ES REST client exposes the status via `ResponseException.getResponse().getStatusLine().getStatusCode()`, so this can be handled entirely inside `convert()` without touching call sites. Making the retryable status set configurable would additionally let operators adapt to a new failure mode without waiting for a release.

Glad to open a PR for this if the approach sounds reasonable.

Contributor guide

Open the contributing guide

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 janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java at convert(), then read IndexTransaction.flushInternal() and BackendOperation.executeDirect() to understand the existing retry path. Use the linked StandardJanusGraph.commit() code to verify failure handling. Done means transient Elasticsearch and transport failures are retried while permanent or unrecognized failures remain permanent.

Written by the indexing model from the issue text.

Assessment

Tech stack
elasticsearch, java
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.