Retry transient Elasticsearch failures instead of dropping the index mutation (#4925)

[batrived]

Fixes #4925.

Problem

ElasticSearchIndex.convert reported every failure except an interrupt as a PermanentBackendException. A commit applies index mutations after the storage mutations, and cannot roll them back. BackendOperation reattempts only a TemporaryBackendException, so a transient failure dropped the mutation after a single attempt and wrote one ERROR log line. The mixed index then stayed inconsistent with the graph until a reindex or a transaction-log recovery repaired it.

A rolling restart, a saturated write queue, or a socket timeout during a GC pause is enough to cause this. The existing retry-error-codes option does not help, because it is empty by default, it acts only inside the Elasticsearch client, and it can match only a failure which produced an HTTP response.

Change

A transient failure is now a TemporaryBackendException, which BackendOperation reattempts with exponential backoff for up to storage.write-time (default 100s). This happens after the retry-limit attempts of the Elasticsearch client are exhausted.

Two new options control the classification. Both are enabled by default:

Option	Default	Purpose
index.[X].elasticsearch.temporary-error-codes	429,502,503,504	HTTP status codes which are transient, from the request itself or from a single bulk item.
index.[X].elasticsearch.temporary-transport-failures	true	Failures which produced no HTTP response: connection refused, connection reset, socket timeout, prematurely closed connection.

Clear both options to keep the previous behavior.

Three further points:

• Bulk item statuses are retained. A bulk request reports item level failures inside a successful HTTP response, so the enclosing response gives no status to classify on. The new ElasticSearchBulkFailureException carries those statuses. The request is reattempted only if every item which failed did so transiently, because a batch which holds a permanently failing item, such as a mapping conflict, cannot succeed on a reattempt.
• The interrupt status is restored. RestElasticSearchClient.performRetryWait wraps an InterruptedException in a RuntimeException, and Thread.sleep had already cleared the status. BackendOperation needs that status to abort its backoff wait, so convert now restores it. Without this the wider cause-chain inspection would let a cancelled commit reissue the bulk request for the whole write time budget.
• Log levels. A transient failure is logged at WARN. ERROR is kept for a mutation which is about to be dropped.

The classification walks the cause chain, because the Elasticsearch client rewraps the failure. I verified against RestClient.extractAndWrapCause in 9.0.3 that the four exception types matched by isTransportFailure cover every transport failure the client produces (ConnectException, SocketTimeoutException, ConnectTimeoutException and ConnectionClosedException), and that both a ResponseException and an InterruptedException reach the caller wrapped.

Reviewers, please note

• The options default to on, so during an Elasticsearch outage every commit which touches a mixed index blocks for up to storage.write-time instead of failing fast. This trades commit latency for index consistency. I am happy to default them off if that trade is not the one you want.
• A reattempt resubmits the whole mutation, including bulk items which already succeeded. That is idempotent for whole-document writes, deletions and SET cardinality properties, but the values of a LIST cardinality property are appended, so they can be duplicated. The changelog and both option descriptions state this. Scoping the reattempt to the failed items only would need the surviving item state to be threaded back out of bulkRequest, which felt out of scope here.

Known gaps, kept out of scope

Happy to fold any of these in, or to open separate issues:

1. IndexTransaction.restore calls index.restore() directly rather than through BackendOperation, unlike flushInternal. So the reindex and transaction-log recovery paths get the new classification but nothing reattempts them.
2. RestElasticSearchClient still logs one ERROR per failed bulk item, which works against the log level change above.
3. A malformed temporary-error-codes value fails graph open with a bare NumberFormatException. The same parse already exists in RestClientSetup, so a shared helper or a ConfigOption verification function would suit both.
4. SolrIndex and LuceneIndex have the same "every failure is permanent" behavior. A backend-agnostic classifier next to BackendOperation would fix all index providers at once.

Separately, and unrelated to this change: MetricInstrumentedIndexProvider.runWithMetrics(BaseTransactionConfigurable, String, StorageRunnable) is missing a return in its !hasGroupName() branch, so mutate and restore run twice. The sibling StorageCallable overload does return impl.call(). It is latent in the default configuration, because metrics.prefix is non-null. I will raise it separately unless you would rather see it here.

---

For all changes:

• Is there an issue associated with this PR? Is it referenced in the commit message?
• Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
• Has your PR been rebased against the latest commit within the target branch (typically master)?
• Is your initial contribution a single, squashed commit?

For code changes:

• Have you written and/or updated unit tests to verify your changes?
• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0? — no new dependencies
• If applicable, have you updated the LICENSE.txt file, including the main LICENSE.txt file in the root of this repository? — not applicable
• If applicable, have you updated the NOTICE.txt file, including the main NOTICE.txt file found in the root of this repository? — not applicable

For documentation related changes:

• Have you ensured that format looks appropriate for the output in which it is rendered? — docs/configs/janusgraph-cfg.md is regenerated by janusgraph-doc, so it matches ConfigurationPrinter output exactly.

[Copilot]

Pull request overview

This PR improves JanusGraph’s Elasticsearch mixed-index write reliability by classifying transient Elasticsearch failures as TemporaryBackendException so that the existing BackendOperation retry loop can reattempt index mutations (instead of dropping them and leaving the index inconsistent).

Changes:

• Add configurable transient-failure classification for Elasticsearch index operations (HTTP status codes + transport failures) and restore thread interrupt status when applicable.
• Preserve bulk item status codes via a dedicated ElasticSearchBulkFailureException so callers can decide whether a bulk failure is entirely transient.
• Add unit tests for exception conversion and bulk-item-status retention; document new options in generated config docs and changelog.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java	Adds transient/permanent classification, new config options, interrupt-status restoration, and WARN vs ERROR logging at the index layer.
janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java	Throws a richer exception for bulk item failures so callers can classify them by item status codes.
janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchBulkFailureException.java	New exception type carrying distinct failed bulk-item HTTP status codes.
janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/ElasticSearchExceptionConversionTest.java	New tests covering cause-chain inspection, status/transport classification, interrupt handling, and retry behavior via BackendOperation.
janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/rest/RestClientRetryTest.java	Adds coverage ensuring failed bulk items retain their status codes.
docs/configs/janusgraph-cfg.md	Documents the two new Elasticsearch retry-classification options.
docs/changelog.md	Adds a changelog entry describing the new retry behavior, defaults, and LIST-cardinality duplication caveat.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.