CAS not working with @Version [DATACOUCH-623]

[spring-projects-issues]

David Prevost opened DATACOUCH-623 and commented

I wanted to use @Version (with long version) to use optimistic locking and kick the CAS of couchbase like mentioned in the documentation.

However, after doing an integrated test, I found out that neither the CASMismatchException or the OptimisticLocking exception are thrown. The problem seems pretty much similar to https://jira.spring.io/browse/DATACOUCH-212.

After searching a bit, it seems that if we want CAS to work, we must use couchbaseOperations.replace... However, the current implement of repositories seems to be using upsert. I was not able to test this theory though.

My integrate test was basically doing:

• Create a User
• Update the token to add one token then save
• Build a new User object with a previous version and a new token and the same id
• Save
• Expecting Optimitics lock error but it is save without error

 

    public void givenExistingUser_whenChangeByTwoPersonsAndSaved_thenOptimisticError() {

        // Given
        final User savedUser = givenExistingUser();

        // When
        // First person
        savedUser.setTokenIds(Set.of(UUID.randomUUID()));
        userSessionDao.save(savedUser);

        // Second person
        final User savedUser2 =
            UserBuilder.builder().userUuid(savedUser.getUserUuid()).build();
        savedUser2.setVersion(savedUser.getVersion() - 1);
        savedUser2.settokenIds(Set.of(UUID.randomUUID()));

        // Then
        final Throwable throwable = catchThrowable(() -> userSessionDao.save(savedUser2));

        assertThat(throwable).isExactlyInstanceOf(CasMismatchException.class);
    }

 

@Document
public class UserEntity {

    @Id
    private UUID userUuid;

    @Version
    private long version;

    @Field
    @NotNull
    private Set<UUID> tokenIds;

 

---

Affects: 4.0.4 (Neumann SR4)

Referenced from: pull request #268

Backported to: 4.0.5 (Neumann SR5)

1 votes, 4 watchers

[spring-projects-issues]

Michael Reiche commented

plum117 - I'm not done looking at this yet, but there was an issue with CAS being ignored.  DATACOUCH-595.  It's merged into master, but nowhere else yet

[spring-projects-issues]

David Prevost commented

Oh okok... when release will test this fix

[spring-projects-issues]

Michael Reiche commented

plum117 - just investigated this further - 595 does not fix it.  The issue is that the save() method of the repository uses upsert()  so that it works for both inserts and update.  And we don't have upsert() doing anything with CAS - there was an internal discussion about this - to the effect that if you know you are replacing an existing document, that replace() should be used. 
Ok - this is exactly what you already stated - "if we want CAS to work, we must use couchbaseOperations.replace... However, the current implement of repositories seems to be using upsert."

replace() does honor CAS - but there is no hook to replace() from the repository.  When (if) that is added, your code would get CasMisMatch wrapped in DataIntegrityViolation like this:

org.springframework.dao.DataIntegrityViolationException: Document has been concurrently modified on the server; nested exception is com.couchbase.client.core.error.CasMismatchException: Document has been concurrently modified on the server {"completed":true,"coreId":"0x7a8fc29100000001","idempotent":false,"lastChannelId":"7A8FC29100000001/0000000052F6AA4F","lastDispatchedFrom":"127.0.0.1:49808","lastDispatchedTo":"127.0.0.1:11210","requestId":11,"requestType":"ReplaceRequest","retried":0,"service":

{"bucket":"70b51006-f5a7-4874-b3dc-64e824dd76e8","collection":"_default","documentId":"1","opaque":"0x14","scope":"_default","type":"kv"}

,"status":"EXISTS","timeoutMs":2500,"timings":{"dispatchMicros":347,"encodingMicros":237,"totalMicros":3881,"serverMicros":0}}

[spring-projects-issues]

jpringle commented

Given that optimistic locking completely depends upon using replace() vs upsert() in the couchbase SDK (there's not even a way to provide a CAS value on an upsert operation, and lack of optimistic lock support is a MAJOR issue, it seems the only reasonable path forward is to add an "update()" operation to the CouchbaseRepository interface.

At any rate, in the meantime the documentation should highlight that the only way to get optimistic locking support is to drop back to CouchbaseTemplate.replaceById()

[spring-projects-issues]

David Prevost commented

I agree with jpringle, there seem to be a major flaw here mostly from what it is documented since from the doc Optimistic locking is supposed to work out of the box just by using @Version 

Either the doc or the code need major review!

[spring-projects-issues]

Michael Reiche commented

jpringle plum117

"it seems the only reasonable path forward is to add an "update()" operation to the CouchbaseRepository interface"

That's the change I made yesterday ( I named it replace() instead of update() ).   Please pull the branch indicated in the pull request ( datacouch_623_add_replace_to_repository_for_cas_usage ) to try it out

[spring-projects-issues]

Mark Paluch commented

From what I understand, the difference in behavior originates from calling replace vs. upsert. It should be the responsibility of the Spring Data infrastructure to determine which method to use instead of propagating this responsibility to the caller. 

Spring Data repositories aim to be technology-agnostic so introducing technology-specific methods are violating that concept.

Instead, the implementation should check whether the entity is versioned and call upon save/saveAll the appropriate Template API method (or the Template API should handle this aspect internally)

[spring-projects-issues]

David Prevost commented

Michael Reiche let me know if you change your implementation following last comment before I test