Delete operation on primary don't do version handling correctly, on 5.3 & 5.x

[bleskes]

I believe this has happened during the back port of #21964 and only relates to the yet to be released 5.3.0

Here's the logic of the delete operation (it's the same both on master and 5.x):

    public static Engine.DeleteResult executeDeleteRequestOnPrimary(DeleteRequest request, IndexShard primary) throws IOException {
        final Engine.Delete delete = primary.prepareDeleteOnPrimary(request.type(), request.id(), request.version(), request.versionType());
        return primary.delete(delete);
    }

Notice how it doesn't do any version conversion for replicas.

On master, this is done in TransportShardBulkAction#executeBulkItemRequest:

case DELETE:
    final DeleteRequest deleteRequest = (DeleteRequest) itemRequest;
    Engine.DeleteResult deleteResult = executeDeleteRequestOnPrimary(deleteRequest, primary);
    if (deleteResult.hasFailure()) {
        response = null;
    } else {
        // update the request with the version so it will go to the replicas
        deleteRequest.versionType(deleteRequest.versionType().versionTypeForReplicationAndRecovery());
        deleteRequest.version(deleteResult.getVersion());
        deleteRequest.setSeqNo(deleteResult.getSeqNo());
        assert deleteRequest.versionType().validateVersionForWrites(deleteRequest.version());
        response = new DeleteResponse(request.shardId(), deleteRequest.type(), deleteRequest.id(), deleteResult.getSeqNo(),
            deleteResult.getVersion(), deleteResult.isFound());
    }

5.x doesn't have that logic in the same method, but rather relies on handling in the execute*OnPrimary method. Here is the relevant code from the index variant:

        if (result.hasFailure() == false) {
            // update the version on request so it will happen on the replicas
            final long version = result.getVersion();
            request.version(version);
            request.versionType(request.versionType().versionTypeForReplicationAndRecovery());
            assert request.versionType().validateVersionForWrites(request.version());
        }

We need to go through the backport again and make 5.x do what master does.

PS. This is yet another proof unit tests are badly needed for this. @dakrone has already started working on it.
PPS. Areek is working on making this entire thing less messy.

[dakrone]

@areek I think this is closed by your PR? unless we want to keep it open until we have unit tests?

[bleskes]

Fixed by #23083