JanusGraph / JanusGraph/janusgraph
Elasticsearch: transient failures classified as PermanentBackendException, so index mutations are silently dropped instead of retried
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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