aerospike / aerospike-client-java

Aerospike Java Client Library
Other
236 stars 212 forks source link

Behavior of MapOperation.removeByKeyList when record is absent #92

Closed Aloren closed 6 years ago

Aloren commented 6 years ago

I have the following test:

    @Autowired 
    private AerospikeClient aerospikeClient;

    @Test
    public void shouldNotRemoveIfNoSuchRecord() throws Exception {
        Key key = new Key(namespace, setName, "document-key");
        List<Value> keys = Arrays.asList(Value.get("key-1"));

        Record record = aerospikeClient.operate(aerospikeClient.writePolicyDefault, key,
                MapOperation.removeByKeyList("field", keys, MapReturnType.VALUE));
    }

Currently it fails with:

com.aerospike.client.AerospikeException: Error Code 12: Bin type error

    at com.aerospike.client.command.ReadCommand.parseResult(ReadCommand.java:105)
    at com.aerospike.client.command.SyncCommand.execute(SyncCommand.java:83)
    at com.aerospike.client.AerospikeClient.operate(AerospikeClient.java:1244)

Is that a correct behavior? I think if the record is absent it is more appropriate to receive KEY_NOT_FOUND_ERROR.

@BrianNichols WDYT?

BrianNichols commented 6 years ago

The current server implementation is a bit complicated. 1) The map remove operation is considered a write operation. 2) The default write policy is to create a record if it does not exist. 3) The record is created with no bins. 4) The map bin (field) does not exist, so the bin type error is generated.

If your WritePolicy.recordExistsAction was UPDATE_ONLY, then you would have received KEY_NOT_FOUND_ERROR.

I brought this issue up to the server team and they are strongly considering changing the default behavior to always return KEY_NOT_FOUND_ERROR when the key is not found.

Aloren commented 6 years ago

Thank you for the explanation. I initially thought that writePolicy for remove operation would only provide timeouts. :) But if it's not true, UPDATE_ONLY will do the trick. If you don't mind I will provide a pull request with the test for this case.

BrianNichols commented 6 years ago

Yes, we would accept a pull request for this test case.

Aloren commented 6 years ago

I've added test case in pull request. Please review. I was using older version of Aerospike, so I asserted on Bin type error.