Reindex can ignore op_type create when using external version

[ismael-hasan]

Elasticsearch version (bin/elasticsearch --version): 6.5.2

Plugins installed: []

JVM version (java -version): Java 10.0.2

OS version (uname -a if on a Unix-like system): Windows 10

Description of the problem including expected versus actual behavior:
In a reindex, when op_type is set as create and we use external versioning we can update documents already existing in the destination index. If op_type is set as create, it is not expected that we can update existing documents; quoting the reindex documentation:

Settings op_type to create will cause _reindex to only create missing documents in the target index. All existing documents will cause a version conflict:

Steps to reproduce:
Create a test index with 1 document:

POST test/doc
{
  "test" : "test"
}

Reindex to another test2 index with op_type create (it will work):

POST _reindex
{
  "source": {
    "index": "test"
  },
  "dest": {
    "index": "test2",
    "op_type": "create",
    "version_type": "external"
  }
}

{
  "took" : 1500,
  "timed_out" : false,
  "total" : 1,
  "updated" : 0,
  "created" : 1,
  "deleted" : 0,
  "batches" : 1,
  "version_conflicts" : 0,
  "noops" : 0,
  "retries" : {
    "bulk" : 0,
    "search" : 0
  },
  "throttled_millis" : 0,
  "requests_per_second" : -1.0,
  "throttled_until_millis" : 0,
  "failures" : [ ]
}

Reindex again to the same index; in this case it will fail due to versioning (it is expected that it fails for op_type create, but not sure if it fails incidentally due to versioning and it is already ignoring the op_type). The request is the same as in the previous step, the response is:

{
  "took": 2,
  "timed_out": false,
  "total": 1,
  "updated": 0,
  "created": 0,
  "deleted": 0,
  "batches": 1,
  "version_conflicts": 1,
  "noops": 0,
  "retries": {
    "bulk": 0,
    "search": 0
  },
  "throttled_millis": 0,
  "requests_per_second": -1,
  "throttled_until_millis": 0,
  "failures": [
    {
      "index": "test2",
      "type": "doc",
      "id": "YjULhGgBIniEFhEZQcaI",
      "cause": {
        "type": "version_conflict_engine_exception",
        "reason": "[doc][YjULhGgBIniEFhEZQcaI]: version conflict, current version [1] is higher or equal to the one provided [1]",
        "index_uuid": "VNd0kK6aSxWCU6v-NO5m8g",
        "shard": "3",
        "index": "test2"
      },
      "status": 409
    }
  ]
}

Reindex again to the same index, but overwriting the version with scripting. In this case, regardless of the document already existing in the destination and op_type being create, it will update that document:

POST _reindex
{
  "source": {
    "index": "test"
  },
  "dest": {
    "index": "test2",
    "op_type": "create",
    "version_type": "external"
  },
    "script": {
    "source": "ctx._version = 6",
    "lang": "painless"
  }
}

{
  "took" : 65,
  "timed_out" : false,
  "total" : 1,
  "updated" : 1,
  "created" : 0,
  "deleted" : 0,
  "batches" : 1,
  "version_conflicts" : 0,
  "noops" : 0,
  "retries" : {
    "bulk" : 0,
    "search" : 0
  },
  "throttled_millis" : 0,
  "requests_per_second" : -1.0,
  "throttled_until_millis" : 0,
  "failures" : [ ]
}

[elasticmachine]

Pinging @elastic/es-core-features

[elasticmachine]

Pinging @elastic/es-distributed

[kwojcicki]

Hey @ismael-hasan what would be a good starting point to start fixing this issue 😄

[stjasink]

Hi @kwojcicki , I was the one who encountered this. @ismael-hasan opened this issue after he helped me diagnose it in a support call.

For me, it was confusing that the op_type of create was being ignored and the reindex task was overwriting documents in the destination index that I did not want it to overwrite. This happened because I had also specified a version_type; I don't know what the reasons were for us using version_type initially, but removing it solved the problem. It took me two days of debugging to figure out exactly what was happening as my tests were quite long turn-around, simulating a real-world situation of concurrent processes.

The way I see it, the op_type is more important than version_type. If op_type is set to create then it should not need to consider versions at all as it should only consider creates and conflicts and no updates.

Perhaps an easy "fix" would be to return an error if version_type is specified when op_type is set to create? Or ignore version_type in that situation, but ignoring that might be confusing for other users.

[ivange94]

but not sure if it fails incidentally due to versioning and it is already ignoring the op_type).

@ismael-hasan you are right about that "op_type": "create" was already been ignored in the first request. The failure was only due to version conflict. If you create a new version of the doc in index test and reindex on test2 which already has that same doc but an older version, it doesn't fail.

Working on the issue.

[ivange94]

@stjasink I'm still tracking down the issue but it sees from this code

https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java#L167-L172

op_type create should not be used with version_type external?

PS: But that's not where the error comes from though. I'm just trying to understand the expected behavior

[stjasink]

@ivange94 That check sounds sensible. Would it make sense to do the same check in https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java to avoid the situation I encountered?