JanusGraph / JanusGraph/janusgraph

Elasticsearch: bulk items failing with HTTP 404 are silently discarded, so updates to a missing document are dropped permanently

Open
#4,926 0 comments 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)
  • Storage Backend: any (behaviour is not backend-specific)
  • Mixed Index Backend: elasticsearch
  • Expected Behavior: a bulk update item that fails with HTTP 404 document_missing_exception should either be surfaced as an error, or be made impossible by always supplying an upsert — so that the mixed index converges.
  • Current Behavior: all bulk items whose status is 404 are filtered out and treated as success. Combined with mutate() supplying a null upsert whenever a mutation contains both deletions and additions, a property change on an element whose Elasticsearch document is missing silently no-ops — permanently, with no exception, no log line, and no metric.
Details

pairErrorsWithSubmittedMutation excludes every 404 from the failure list, regardless of the request type that produced it:

https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java#L464-L482

if (item.getError() != null && item.getStatus() != HttpStatus.SC_NOT_FOUND) {
    errors.add(Triplet.with(item.getError(), item.getStatus(), submittedBulkRequestItems.get(itemIndex)));
}

The exemption is correct for delete items — deleting an already-absent document should be idempotent. But update items are conflated with them, and for an update a 404 means document_missing_exception: the write did not happen.

That matters because of how mutate() builds the requests. When a mutation has both deletions and additions, the upsert document is deliberately withheld:

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

final Map upsert;
if (!mutation.hasDeletions()) {
    upsert = getNewDocument(mutation.getAdditions(), information.get(storeName));
} else {
    upsert = null;
}

Changing the value of a SINGLE-cardinality indexed property is exactly this case: it produces a deletion of the old value and an addition of the new one against the same document id, with isNew == false. Both become update operations, hasDeletions() is true, so upsert is null. If the document is absent, both items return 404, both are discarded, and the property is never indexed.

The failure is self-perpetuating: every subsequent update to that element takes the same path, so the document is never recreated. The element stays permanently invisible to the mixed index while present in the storage backend, and nothing in the write path reports it — IndexProvider.mutate returns normally, so even the indexProvider.<name>.mutate.exceptions metric stays at zero.

Steps to Reproduce
  1. Configure a graph with an Elasticsearch mixed index over a SINGLE-cardinality property key.
  2. Add a vertex with that property and commit. Confirm the document exists and the vertex is findable through the mixed index.
  3. Delete the document directly from Elasticsearch (DELETE /<index>/_doc/<id>) to simulate an index write that was lost earlier.
  4. Change the property's value on that vertex and commit.
  5. Observe: the bulk response contains 404 document_missing_exception for the items, no exception is raised, and the vertex is still not findable through the mixed index.
  6. Repeat step 4 any number of times — the document is never recreated.
Suggested Fix

Any of these, in rough order of preference:

  1. Limit the 404 exemption to delete request types. RequestBytes is constructed from an ElasticSearchMutation and already reads request.getRequestType(), but does not retain it — carrying it on RequestBytes would make the request type available in pairErrorsWithSubmittedMutation.
  2. Always supply the upsert document in mutate() (drop the hasDeletions() conditional), so an update against a missing document creates it. The deletion script is added to the same bulk ahead of the addition, and Elasticsearch applies bulk items to a given document id in submission order, so ordering is preserved.
  3. At minimum, count and log discarded 404 items so that the condition is observable rather than silent.

Happy to open a PR for (1) and (2) if that 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/rest/RestElasticSearchClient.java at pairErrorsWithSubmittedMutation, then trace RequestBytes and ElasticSearchMutation request types. Compare that path with mutate() in janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java. Done means missing-document update failures are reported or recreated, while already-absent deletes remain idempotent.

Written by the indexing model from the issue text.

Assessment

Tech stack
elasticsearch, java
Domain
databases, search
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.