elasticsearch/elasticsearch — pull request #2282 by imotov (2012-09-23 22:11:01+00:00)
Title: Fix possible race condition when routing is required but not specified f...

...or a delete operation

This small pull request fixes a possible race condition in the TransportShardBulkAction. This race condition can be triggered by a bulk delete operation on an index with required routing when several primary shards are located on the node that executes the operation. It sounds complicated, but it can be easily reproduced by placing content of AliasRoutingTests.testRequiredRoutingMappingWithAlias or
SimpleRoutingTests.testRequiredRoutingMapping into an infinite loop. On my machine it typically fails after only a few iterations.

When an index is marked as routing required but routing is not specified on a delete operation, TransportBulkAction broadcasts such delete operation to all shards using [the same delete request object](https://github.com/elasticsearch/elasticsearch/blob/master/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java#L191).

When TransportShardBulkAction receives this request on a primary shard, it tries to delete the record and [updates the version field in the request](https://github.com/elasticsearch/elasticsearch/blob/master/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java#L210). The problem occurs because the version can be updated even if the record on the shard was not actually deleted. It looks like this behavior is desired and was introduced by #1341. When several primary shards are located on the same node, the DeleteRequest object is shared among all shards and the version changes on one shard affect all shards of the index on the same node. As a result the delete operation sometimes fails with VersionConflictEngineException.

While this pull request fixes the problem, it might still make sense to not share the DeleteRequest among shards. What do you think?