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

[batrived]

• 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:

janusgraph/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java

Lines 464 to 482 in ac0eb23

	private List<Triplet<Object, Integer, RequestBytes>> pairErrorsWithSubmittedMutation(
	//Bulk API is documented to return bulk item responses in the same order of submission
	//https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html#bulk-api-response-body
	//As such we only need to retry elements that failed
	final List<Map<String, RestBulkResponse.RestBulkItemResponse>> bulkResponseItems,
	final List<RequestBytes> submittedBulkRequestItems) {
	final List<Triplet<Object, Integer, RequestBytes>> errors = new ArrayList<>(bulkResponseItems.size());
	for (int itemIndex = 0; itemIndex < bulkResponseItems.size(); itemIndex++) {
	Collection<RestBulkResponse.RestBulkItemResponse> bulkResponseItem = bulkResponseItems.get(itemIndex).values();
	if (bulkResponseItem.size() > 1) {
	throw new IllegalStateException("There should only be a single item per bulk reponse item entry");
	}
	RestBulkResponse.RestBulkItemResponse item = bulkResponseItem.iterator().next();
	if (item.getError() != null && item.getStatus() != HttpStatus.SC_NOT_FOUND) {
	errors.add(Triplet.with(item.getError(), item.getStatus(), submittedBulkRequestItems.get(itemIndex)));
	}
	}
	return errors;
	}

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:

janusgraph/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java

Lines 875 to 881 in ac0eb23

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

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.