elasticsearch-java icon indicating copy to clipboard operation
elasticsearch-java copied to clipboard

new Java API client throws org.elasticsearch.client.ResponseException when there is a version conflict

Open frank-montyne opened this issue 2 years ago • 1 comments

Elasticsearch Version

7.17.3

Installed Plugins

No response

Java Version

bundled

OS Version

macOS Montery

Problem Description

When adding 2 documents with the same id to an index I get a org.elasticsearch.client.ResponseException instead of the new co.elastic.clients.elasticsearch._types.ElasticsearchException. That is very confusing when working with the new Java AOI client. Seems like a bug.

Example code:

private boolean addLock(Lock distributedLock) throws Exception {
		try {
			// A version conflict status exception will be thrown if the document already exists for the same id.
			if (logger.isDebugEnabled()) logger.debug(String.format("Adding distributed lock %s -> %s", getThreadInfo(), distributedLock.toJson()));
			
			elasticSearch.getESClient().index(builder -> 
				builder
					.index(lockIndex)
					.id(distributedLock.id())
					.withJson(distributedLock.toESJsonReader())
					.opType(OpType.Create)
					.waitForActiveShards(asBuilder -> asBuilder.count(1))
					.refresh(Refresh.True));
			
			if (logger.isDebugEnabled()) logger.debug(String.format("Distributed lock added %s -> %s", getThreadInfo(), distributedLock.toJson()));
			return true;
		}
		// Seems like a bug in ElasticSearch to throw an org.elasticsearch.client.ResponseException instead of the new co.elastic.clients.elasticsearch._types.ElasticsearchException!
		// Catching both for the moment being.
		catch (ResponseException e) {
			// Only version conflict exceptions are allowed.
			if (e.getResponse().getStatusLine().getStatusCode() != RestStatus.CONFLICT.getStatus()) throw e;
			
			// ElasticSearch version conflict. Another host/process/thread already performed an action on this lock.
			if (logger.isDebugEnabled()) logger.debug(String.format("Failed to add distributed lock. ES version conflict %s -> %s", getThreadInfo(), distributedLock.toJson()));
			return false;
		}
		catch (ElasticsearchException e) {
			// Only version conflict exceptions are allowed.
			if (e.response().status() != RestStatus.CONFLICT.getStatus()) throw e;
			
			// ElasticSearch version conflict. Another host/process/thread already performed an action on this lock.
			if (logger.isDebugEnabled()) logger.debug(String.format("Failed to add distributed lock. ES version conflict %s -> %s", getThreadInfo(), distributedLock.toJson()));
			return false;
		}
	}

Steps to Reproduce

Add 2 document with the same id

Logs (if relevant)

No response

frank-montyne avatar May 12 '22 15:05 frank-montyne

I also noticed that. In my opinion, this is exactly the opposite of what the official documentation describes. There it says:

  • Requests that were received by the Elasticsearch server but that were rejected ... produce an ElasticsearchException
  • Requests that failed to reach the server ... will be a ResponseException (in case of RestClientTransport)

Noticing a version conflict can only be determined once the request has reached Elasticsearch. Hence, this would be an ElasticsearchException. If I see the current implementation of RestClientTransport correctly (only had a brief look), only responses having a status code of 400,401,403,404 or 405 can even be thrown as an ElasticsearchException. Conflicts (409) are not in that list.

Is there any plan to adapt this soon? I personally think it would be nice, because think it makes more sense. However, this is a breaking change for us because our code does always check for both ResponseExceptions and ElasticsearchExceptions.

kemusag avatar Dec 16 '22 14:12 kemusag