Topics

Java Community | Help. Code. Learn.•5w ago

handling lost update problem happening with one thread updating a resource at a time

so i got this quite an interesting problem where i got a piece of code that edits a entity there is one class and a bunch of other classes that inherit it the superclass has a version field for optimistic locking all subclasses are stored in the same jpa repository the lost update/stale data problem comes when sending an update command example scenario employee gets updated salary to 3000 but the request will sent later employee gets updated salary to 5000 but the request is sent immediately so the employee gets 3000 salary even though he was supposed to have 5000 i was thinking of comparing the version of the entity and the version thats sent in the command but that generates race condition example code

    @Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

    @Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

123 Replies

JavaBot•5w ago

⌛ This post has been reserved for your question.

Hey @ayylmao123xdd! Please use /close or the Close Post button above when your problem is solved. Please remember to follow the help guidelines. This post will be automatically marked as dormant after 300 minutes of inactivity.

TIP: Narrow down your issue to simple and precise questions to maximize the chance that others will reply in here.

dan1st•5w ago

Did you try logging the SQL statements?

ayylmao123xddOP•5w ago

yea the version gets increased every time properly i need some way to check if the version in the command and the entity equal and if not throw an exception without a race condition and also cant write a custom query for updating so that narrows down most solutions and also cant add a parameter for fetching the subclass in database because then it wont throw the optimistic lock exception when 2 threads try to edit the subclass at once

dan1st•5w ago

What is your service doing?

ayylmao123xddOP•5w ago

edits a subclass so for example if there is salary you can edit it to 5000 or whatever

dan1st•5w ago

Can you show it?

ayylmao123xddOP•5w ago

and the update method just has a bunch of setters 1 sec

@Mapper(componentModel = "spring", nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE,
        unmappedTargetPolicy = ReportingPolicy.IGNORE, uses = {SuperClassMapperConfig.class})
public interface SubclassMapper {
    Subclass mapFromCommand(CreateSubclassCommand command);

    Dto mapToDto(Subclass subclass);

    @InheritConfiguration(name = "update")
    void edit(EditSubclassCommand source, @MappingTarget Subclass Subclass);
}

@Mapper(componentModel = "spring", nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE,
        unmappedTargetPolicy = ReportingPolicy.IGNORE, uses = {SuperClassMapperConfig.class})
public interface SubclassMapper {
    Subclass mapFromCommand(CreateSubclassCommand command);

    Dto mapToDto(Subclass subclass);

    @InheritConfiguration(name = "update")
    void edit(EditSubclassCommand source, @MappingTarget Subclass Subclass);
}

its just a mapstruct edit method

dan1st•5w ago

I was asking about the service method

ayylmao123xddOP•5w ago

that sets new values to the entity

dan1st•5w ago

service.update()

ayylmao123xddOP•5w ago

oh it just calls the edit method in the mapper let me show

private Dto update(EditSubclassCommand command, Subclass subclass) {
        UpdateStrategy strategy = strategies.get(command.getType());
        return strategy.update(command, subclass);
    }

private Dto update(EditSubclassCommand command, Subclass subclass) {
        UpdateStrategy strategy = strategies.get(command.getType());
        return strategy.update(command, subclass);
    }

strategies is just a map for all the mappers depending on the subclass type iedited the classname of this was trying with student class but didnt work

dan1st•5w ago

Can you show the relevant UpdateStrategy?

ayylmao123xddOP•5w ago

sure

dan1st•5w ago

also that

ayylmao123xddOP•5w ago

1 second yes i tried sql statements but it wont work in my case since all the subclasses are stored in one repository and i havent heard of a way to dynamically update the edit method and i cant change the repository file so i would need a way to inject an update method into the repository

public class UpdateStrategy implements SuperclassStrategy {
    private final SubclassMapper mapper;

    @Override
    public void update(EditSuperclassCommand command, Superclass superclass) {
        mapper.edit((EditSubclassCommand) command, (Subclass) superclass);
    }
}

public class UpdateStrategy implements SuperclassStrategy {
    private final SubclassMapper mapper;

    @Override
    public void update(EditSuperclassCommand command, Superclass superclass) {
        mapper.edit((EditSubclassCommand) command, (Subclass) superclass);
    }
}

its just this it calls the mapper i sent also has a bunch of irrelevant methods for this so i didnt include them

dan1st•5w ago

So nothing in there is doing any DB update? Then why should it change anything?

ayylmao123xddOP•5w ago

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

this update does the update and the mapper because if you set the values of the entity then it automatically gets saved because there is transactional so you dont need to call

repository.save(yourClass)

repository.save(yourClass)

but yes the entity gets updated and saved to database i checked with a bunch of tests

dan1st•5w ago

no...

ayylmao123xddOP•5w ago

yes

dan1st•5w ago

that isn't how @Transactional works

ayylmao123xddOP•5w ago

i got a bunch of tests and it works well and i dont use

java repository.save(yourClass)

java repository.save(yourClass)

in the code

dan1st•5w ago

then it doesn't save it to the DB if your tests use the same object, then the tests are not checking the DB

ayylmao123xddOP•5w ago

ok so in the test it creates an entity saves it to db and saves the id of the saved entity sends request update update method does its magic db grabs the entity again based on id compares if stuff has changed so yes it does save and update properly im just stuck with this problem with versioning i have to somehow compare version without using if

dan1st•5w ago

if you don't interact with any repository, @Transactional does nothing

ayylmao123xddOP•5w ago

yes but the entity still is saved to database after the transaction ends with the changes from the command

dan1st•5w ago

Then where are you saving it?

ayylmao123xddOP•5w ago

in the repository it saves itself automatically

dan1st•5w ago

Then show the code where you are saving it

ayylmao123xddOP•5w ago

wait let me grab the name of why it happens

dan1st•5w ago

@Transactional doesn't do it automatically except you have extra code doing stuff like that, e.g. using AOP

ayylmao123xddOP•5w ago

so the entity is tracked in the persistence context and at the end of the transaction hibernate just saves the entity back to db again i use jpa by the way maybe that explains why it works in my case

dan1st•5w ago

not automatically unless you did something for that

ayylmao123xddOP•5w ago

no i never call a save method

dan1st•5w ago

and that's why it doesn't save anything But I think I might know why the test seems like it is saved So in the test, I think the following happens: - You create the entity - You save it to the DB - Update changes the fields on the object but it's never saved in the DB - You retrieve the entity again but it gives you the same object - Since it's the same object, it has the changes even though these changes are not in the DB You can enable query logging to see that or check the DB after making the request

ayylmao123xddOP•5w ago

yea i got sql logging enabled but my question is how do i add version checking like in here

  @Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

  @Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

just without race condition i need some way to compare if the version in command

dan1st•5w ago

using spring.jpa.show-sql=true and/or spring.jpa.hibernate.show-sql=true?

ayylmao123xddOP•5w ago

is the same one as the one in the database yes both

dan1st•5w ago

if yes then show the logs of just calling that edit method

ayylmao123xddOP•5w ago

sure but let me rephrase maybe lost update happens because another request overried changes of the other request because there is no comparing versions between the command and the entity from the database now let me show logs

dan1st•5w ago

yeah because it isn't even in the DB the logs of only the edit() method

ayylmao123xddOP•5w ago

o logs aside check this out i added statistics to measure the amount of requests sent to database and in each test it shows 2 which means it grabs the entity from db then saves it let me paste logs

Hibernate: select p1_0.id,p1_0.type (bunch of fields here) from superclass p1_0 where p1_0.id=?
Hibernate: update superclass set (bunch of fields here) where id=? and version=?

Hibernate: select p1_0.id,p1_0.type (bunch of fields here) from superclass p1_0 where p1_0.id=?
Hibernate: update superclass set (bunch of fields here) where id=? and version=?

as you can see it calls the update

dan1st•5w ago

Which logs are that? and when are these logs printed?

ayylmao123xddOP•5w ago

after calling the edit method

dan1st•5w ago

Are these logs printed when only doing a request calling edit?

ayylmao123xddOP•5w ago

in my integration test yea only then

dan1st•5w ago

outside of an integration test

ayylmao123xddOP•5w ago

it doesnt call the update query when saving in for example the save method so yes it does save to database i need to figure out how to do this comparing versions

dan1st•5w ago

I can show you an example:

@SpringBootApplication
public class SpringJpaTestApplication {

    public static void main(String[] args) {
        SpringApplication.run(SpringJpaTestApplication.class, args);
    }

    private final FirstEntityRepository firstRepo;
    private final Test test;
    
    public SpringJpaTestApplication(FirstEntityRepository firstRepo, Test test) {
        this.firstRepo = firstRepo;
        this.test = test;
    }
    
    @EventListener
    public void run(ApplicationReadyEvent e) {
        FirstEntity f = new FirstEntity();
        f.setId("a");
        f.setName("b");
        firstRepo.save(f);
        
        test.doSomething(f);

    }
}

@Component
class Test {
    @Transactional
    public void doSomething(FirstEntity f) {
        f.setName("new name");
    }
}

@SpringBootApplication
public class SpringJpaTestApplication {

    public static void main(String[] args) {
        SpringApplication.run(SpringJpaTestApplication.class, args);
    }

    private final FirstEntityRepository firstRepo;
    private final Test test;
    
    public SpringJpaTestApplication(FirstEntityRepository firstRepo, Test test) {
        this.firstRepo = firstRepo;
        this.test = test;
    }
    
    @EventListener
    public void run(ApplicationReadyEvent e) {
        FirstEntity f = new FirstEntity();
        f.setId("a");
        f.setName("b");
        firstRepo.save(f);
        
        test.doSomething(f);

    }
}

@Component
class Test {
    @Transactional
    public void doSomething(FirstEntity f) {
        f.setName("new name");
    }
}

this runs

Hibernate: select fe1_0.id,fe1_0.name,o1_0.other_id from first_entity fe1_0 left join second_entity o1_0 on fe1_0.id=o1_0.other_id where fe1_0.id=?
Hibernate: insert into first_entity (name,id) values (?,?)

Hibernate: select fe1_0.id,fe1_0.name,o1_0.other_id from first_entity fe1_0 left join second_entity o1_0 on fe1_0.id=o1_0.other_id where fe1_0.id=?
Hibernate: insert into first_entity (name,id) values (?,?)

no UPDATE

ayylmao123xddOP•5w ago

yea because it saves you are calling the save method in my case it just updates ill send you the example code i use once i figure out this versioning problem

dan1st•5w ago

otherwise it wouldn't insert it

ayylmao123xddOP•5w ago

yes it wouldnt insert without the repo save method when saving but when editing it saves automatically

dan1st•5w ago

The result of f.setName is never stored in the DB in my example

ayylmao123xddOP•5w ago

ill send you the project that shows how it works in my case with some changed values so i dont leak some stuff actually you can just write a method that grabs the entity from database and updates some field and then dont call any save method and see if it gets saved in an integration test ill send the project anyway probably tomorrow as today i want to finish this versioning problem

dan1st•5w ago

If I do it like that

package io.github.danthe1st.spring_jpa_test;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.event.ApplicationReadyEvent;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

@SpringBootApplication
public class SpringJpaTestApplication {

    public static void main(String[] args) {
        SpringApplication.run(SpringJpaTestApplication.class, args);
    }

    private final FirstEntityRepository firstRepo;
    private final Test test;
    
    public SpringJpaTestApplication(FirstEntityRepository firstRepo, Test test) {
        this.firstRepo = firstRepo;
        this.test = test;
    }
    
    @EventListener
    public void run(ApplicationReadyEvent e) {
        FirstEntity f = new FirstEntity();
        f.setId("a");
        f.setName("b");
        firstRepo.save(f);
        
        test.doSomething(f);
        
        System.out.println(firstRepo.getNameById(f.getId()));

    }
}

@Component
class Test {
    @Transactional
    public void doSomething(FirstEntity f) {
        f.setName("new name");
    }
}

package io.github.danthe1st.spring_jpa_test;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.event.ApplicationReadyEvent;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

@SpringBootApplication
public class SpringJpaTestApplication {

    public static void main(String[] args) {
        SpringApplication.run(SpringJpaTestApplication.class, args);
    }

    private final FirstEntityRepository firstRepo;
    private final Test test;
    
    public SpringJpaTestApplication(FirstEntityRepository firstRepo, Test test) {
        this.firstRepo = firstRepo;
        this.test = test;
    }
    
    @EventListener
    public void run(ApplicationReadyEvent e) {
        FirstEntity f = new FirstEntity();
        f.setId("a");
        f.setName("b");
        firstRepo.save(f);
        
        test.doSomething(f);
        
        System.out.println(firstRepo.getNameById(f.getId()));

    }
}

@Component
class Test {
    @Transactional
    public void doSomething(FirstEntity f) {
        f.setName("new name");
    }
}

then it prints "b" and not "new name" Because the

    @Transactional
    public void doSomething(FirstEntity f) {
        f.setName("new name");
    }

    @Transactional
    public void doSomething(FirstEntity f) {
        f.setName("new name");
    }

doesn't save anything if you want it to be saved, you need a save() If you don't believe it, make an actual HTTP request outside of an integration test that only triggers this method and check the result in the DB or give me a minimal example

ayylmao123xddOP•5w ago

1 second then @dan1st | Daniel done sorry for the long wait

package com.example.demo;

import lombok.RequiredArgsConstructor;
import org.springframework.boot.CommandLineRunner;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
@RequiredArgsConstructor
public class DemoApplication implements CommandLineRunner {
    private final exampleservice exampleservice;

    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }

    @Override
    public void run(String... args) throws Exception {
        exampleservice.doStuff();
        exampleservice.doStuff2();
        exampleservice.doStuff3();
    }
}

package com.example.demo;

import lombok.RequiredArgsConstructor;
import org.springframework.boot.CommandLineRunner;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
@RequiredArgsConstructor
public class DemoApplication implements CommandLineRunner {
    private final exampleservice exampleservice;

    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }

    @Override
    public void run(String... args) throws Exception {
        exampleservice.doStuff();
        exampleservice.doStuff2();
        exampleservice.doStuff3();
    }
}

package com.example.demo;

import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@RequiredArgsConstructor
@Service
public class exampleservice {
    private final examplerepository examplerepository;

    @Transactional
    public void doStuff() {
        Example example = Example.builder()
                .firstName("name")
                .build();

        examplerepository.save(example);
    }

    @Transactional
    public void doStuff2() {
        Example example = examplerepository.findById(1L).orElseThrow();
        example.setFirstName("name2");
    }

    @Transactional
    public void doStuff3() {
        Example example = examplerepository.findById(1L).orElseThrow();
        System.out.println(example.getFirstName());
    }
}

package com.example.demo;

import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@RequiredArgsConstructor
@Service
public class exampleservice {
    private final examplerepository examplerepository;

    @Transactional
    public void doStuff() {
        Example example = Example.builder()
                .firstName("name")
                .build();

        examplerepository.save(example);
    }

    @Transactional
    public void doStuff2() {
        Example example = examplerepository.findById(1L).orElseThrow();
        example.setFirstName("name2");
    }

    @Transactional
    public void doStuff3() {
        Example example = examplerepository.findById(1L).orElseThrow();
        System.out.println(example.getFirstName());
    }
}

Hibernate: insert into example (first_name) values (?)
Hibernate: select e1_0.id,e1_0.first_name from example e1_0 where e1_0.id=?
Hibernate: update example set first_name=? where id=?
Hibernate: select e1_0.id,e1_0.first_name from example e1_0 where e1_0.id=?
name2

Hibernate: insert into example (first_name) values (?)
Hibernate: select e1_0.id,e1_0.first_name from example e1_0 where e1_0.id=?
Hibernate: update example set first_name=? where id=?
Hibernate: select e1_0.id,e1_0.first_name from example e1_0 where e1_0.id=?
name2

as you can see it perfectly edits the entity without calling a save method to save it back to db excuse my class names i was writing it quite fast i still havent found a solution on how to compare version without race condition weirdest task i got tbh cant write a custom query to check versioning cant fetch entity based on version and cant compare version with command using a if statement

dan1st•5w ago

oh you are receiving in the transactional method?

ayylmao123xddOP•5w ago

yes thats how it was in both the tests and normal code

dan1st•5w ago

I didn't notice that, sry

ayylmao123xddOP•5w ago

no problem do you have any idea how to do this part though

dan1st•5w ago

Are you specifying optimistic locking in the transaction?

ayylmao123xddOP•5w ago

yes it uses optimistic locking with @version annotation on entity but optimistic locking wont prevent a lost update when its only 1 thread modifying data so i need to compare version between the command and the entity fetched from db somehow without causing race condition

dan1st•5w ago

Where are you specifying that?

ayylmao123xddOP•5w ago

in the fetch method

@Lock(value = LockModeType.OPTIMISTIC_FORCE_INCREMENT)
    Optional<Superclass> findWithLockingById(long id);

@Lock(value = LockModeType.OPTIMISTIC_FORCE_INCREMENT)
    Optional<Superclass> findWithLockingById(long id);

as you can see

dan1st•5w ago

ah ok

ayylmao123xddOP•5w ago

yea i was thinking of using a second request to check the version in the edit method but thats so unoptimized

dan1st•5w ago

Can you show the SQL logs of both updates where the second doesn't get through?

ayylmao123xddOP•5w ago

the problem is that it does go through here

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

if i dont add that if statement the update will always go through even if the data that i send in the request is outdated so i need to somehow do the if statement without race condition or a solution i havent thought of yet because natively the versioning just increases version of the entity without comparing it to the editcommand

dan1st•5w ago

yeah but I want to see the SQL logs to see what exactly happens you said it would happen with the methods being executed one after each other

ayylmao123xddOP•5w ago

ok let me launch the app

dan1st•5w ago

Can you log the contents (both version and the relevant fields) after the service.update as well as SQL logging? and then show the logs of both requests as well as the result

ayylmao123xddOP•5w ago

Hibernate: select p1_0.id,p1_0.type (fields) from superclass p1_0 where p1_0.id=?
0 -- current version
Hibernate: update superclass set (fields) where id=? and version=?
Hibernate: select p1_0.id,p1_0.type (fields) where p1_0.id=?
1 -- current version
Hibernate: update superclass set (fields) where id=? and version=?

Hibernate: select p1_0.id,p1_0.type (fields) from superclass p1_0 where p1_0.id=?
0 -- current version
Hibernate: update superclass set (fields) where id=? and version=?
Hibernate: select p1_0.id,p1_0.type (fields) where p1_0.id=?
1 -- current version
Hibernate: update superclass set (fields) where id=? and version=?

there i added a print that says the current version after fetching it from database it was the code without the version checking

dan1st•5w ago

Can you show that? Can you check whether the fields are actually modified? using a debugger

ayylmao123xddOP•5w ago

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            System.out.println(subclass.getVersion() + " -- current version");
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            System.out.println(subclass.getVersion() + " -- current version");
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

just here yes they are modified version is raised but how could i check the version from command and the version from db

dan1st•5w ago

oh you had set fields What are the values here?

ayylmao123xddOP•5w ago

yes its about this edit command version

dan1st•5w ago

the values of the relevant fields

ayylmao123xddOP•5w ago

id is 1 type is irrelevant i guess and version in first update is 0 and in second update is 1

dan1st•5w ago

I meant the field you want to change but isn't actually changed

ayylmao123xddOP•5w ago

no no no

dan1st•5w ago

?

ayylmao123xddOP•5w ago

the fields get changed

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            System.out.println(subclass.getVersion() + " -- current version");
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
//            if (command.getVersion() != subclass.getVersion()) {
//            } generates race condition
            System.out.println(subclass.getVersion() + " -- current version");
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

just this commented out code i need to check whether the get version from command is the same as subclass command otherwise throw an exception all the other stuff works as intended

dan1st•5w ago

Doesn't JPA do that automatically if optimistic locking fails?

ayylmao123xddOP•5w ago

it does that when two threads modify an entity at once but in this case we assume the data in editcommand is outdated hence why it has its own version field

dan1st•5w ago

Actually why do you have a command and a subclass version?

ayylmao123xddOP•5w ago

to be compared with the entity field for this because we assume the data in edit command might come outdated so it needs to compare the version with db entity to make sure its not outdated

dan1st•5w ago

The if here doesn't generate a race condition assuming you are throwing an exception here. If the version changes in the meantime, JPA will throw an exception when the method exits

ayylmao123xddOP•5w ago

if its outdated just an exception

dan1st•5w ago

the

if (command.getVersion() != subclass.getVersion()) {
  throw new SomeRuntimeException();
}

if (command.getVersion() != subclass.getVersion()) {
  throw new SomeRuntimeException();
}

should work but only if the exception is a RuntimeException actually no it should work in any way it doesn't need to rollback

ayylmao123xddOP•5w ago

yea it throws a runtime exception but a senior dev told me to not do that if statement since its a race condition but didnt want to tell what to use instead lol hence my question here

dan1st•5w ago

If the command.getVersion() != subclass.getVersion() check decides they are equal and it's edited in the meantime, Spring will abort it due to its optimistic locking so no there shouldn't be a race condition but this is not at all obvious so it needs to be documented properly WHY this is safe and you should maybe also add a test checking that there is really no race condition so if something changes in the future, you hopefully get alerted

ayylmao123xddOP•5w ago

ok wait let me pull up the notes i got "lots update may occur when there is just one request if the entity version is 8 and you send a request with version 7 then an exception should be thrown it should be handled with versioning not if statement if statements generate race condition" thats what i got so i assumed using an if statement just cant be used in this case maybe it doesnt generate race condition in that case ill have to somehow show the senior how hes wrong or just look for another solution thanks for your help @dan1st | Daniel

JavaBot•5w ago

If you are finished with your post, please close it. If you are not, please ignore this message. Note that you will not be able to send further messages here after this post have been closed but you will be able to create new posts.

dan1st•5w ago

then you would need to argue why there is no race condition, yes I think it wasn't clear that you are using both an optimistic lock and the if and include that in the form of a comment in the method showing how the combination of the two versions prevent the race condition

ayylmao123xddOP•5w ago

yes maybe he just didnt see that i use optimistic locking unsure though

dan1st•5w ago

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
            //if the version doesn't match the version from the command, reject the command
            //this protects against modifications from within the command
            //Concurrent modifications after the checks are handled with an optimistic lock from repository.findWithLockingById(id) which is checked with the transaction
            if (command.getVersion() != subclass.getVersion()) {
              throw new InvalidVersionException();
            }
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

@Transactional
    public Dto edit(long id, EditCommand command) {
        try {
            Subclass subclass = repository.findWithLockingById(id)
                    .orElseThrow();
            //if the version doesn't match the version from the command, reject the command
            //this protects against modifications from within the command
            //Concurrent modifications after the checks are handled with an optimistic lock from repository.findWithLockingById(id) which is checked with the transaction
            if (command.getVersion() != subclass.getVersion()) {
              throw new InvalidVersionException();
            }
            service.update(command, subclass);
            return mapper.mapToDto(subclass);
    }

ayylmao123xddOP•5w ago

o thanks for the description

JavaBot•5w ago

If you are finished with your post, please close it. If you are not, please ignore this message. Note that you will not be able to send further messages here after this post have been closed but you will be able to create new posts.

ayylmao123xddOP•5w ago

as again thanks for the time spent on this

dan1st•5w ago

something along the lines of that, you might want to change it in some form or the other np

ayylmao123xddOP•5w ago

i guess it all came down to just uncommenting the if statement lol too much thinking on the problem

dan1st•5w ago

Also you could consider the if as some form of input validation it checks whether the user has the most recent value while optimistic locking checks for other issues Oh and I recommend also including something that the version check MUST be after obtaining the obtimistic lock and it should compare to the optimistic lock

ayylmao123xddOP•5w ago

oh i use java validations for that it checks whether the command has proper fields even before reaching the service

dan1st•5w ago

I meant it being some other type of validation but make sure to properly handle the exception from the user btw

ayylmao123xddOP•5w ago

yes i got the advice controller and all that fancy stuff for handling errors

dan1st•5w ago

and you might also write a test that ensures the optimistic locking actually works

ayylmao123xddOP•5w ago

i got those too now i just gotta add another one where race condition happens and to actually make sure it doesnt go through

dan1st•5w ago

yeah that's what I mean

ayylmao123xddOP•5w ago

when i send 2 threads at once it for sure throws an error so thats a positive thing

dan1st•5w ago

Something like

Subclass yourSubclass;//make sure it's in the DB

//make sure the mapper is mocked
CountDownLatch waitForRaceLatch = new CountDownLatch(1);
CountDownLatch waitForMapperCallLatch = new CountDownLatch(1);

Command commandThatShouldFail = new Command(123);
Command winnerCommand = new Command(456);
when(mapper.update(commandThatShouldFail, any())).then(invocation -> {
  waitForMapperCallLatch.countDown();
  waitForRaceLatch.await();
  return null;
});
when(mapper.update(winnerCommand, any()).then(() -> {});
Thread winnerThread = new Thread(() -> {
  waitForMapperCallLatch.await();
  yourService.edit(yourSubclass.getId(), winnerCommand);
  waitForRaceLatch.countDown();
});
winnerThread.start();
assertThrows(RuntimeException.class, () -> yourService.edit(yourSubclass.getId(), commandThatShouldFail));//TODO use the actual exception thrown by spring

//ensure thread really stops
winnerThread.interrupt();
winnerThread.join();

Subclass yourSubclass;//make sure it's in the DB

//make sure the mapper is mocked
CountDownLatch waitForRaceLatch = new CountDownLatch(1);
CountDownLatch waitForMapperCallLatch = new CountDownLatch(1);

Command commandThatShouldFail = new Command(123);
Command winnerCommand = new Command(456);
when(mapper.update(commandThatShouldFail, any())).then(invocation -> {
  waitForMapperCallLatch.countDown();
  waitForRaceLatch.await();
  return null;
});
when(mapper.update(winnerCommand, any()).then(() -> {});
Thread winnerThread = new Thread(() -> {
  waitForMapperCallLatch.await();
  yourService.edit(yourSubclass.getId(), winnerCommand);
  waitForRaceLatch.countDown();
});
winnerThread.start();
assertThrows(RuntimeException.class, () -> yourService.edit(yourSubclass.getId(), commandThatShouldFail));//TODO use the actual exception thrown by spring

//ensure thread really stops
winnerThread.interrupt();
winnerThread.join();

but you'd need to make sure your testing environment actually allows multithreaded testing alternatively you can try doing it in a singlethreaded with with Propagation.REQUIRES_NEW the idea of that test would be that it enforces a race condition

ayylmao123xddOP•5w ago

yea i did almost the same thing

dan1st•5w ago

I also thought about using a single CyclicBarrier instead of the two CountDownLatches xd

ayylmao123xddOP•5w ago

actually i just created 2 threads started them and joined and i added a catch for both of them atomic boolean and if one of them throws an error then it means the code worked since it stopped one of the threads from editing data

dan1st•5w ago

the idea here is that it enforces the exception from optimistic locking

ayylmao123xddOP•5w ago

yes in my case too

dan1st•5w ago

not the other one

ayylmao123xddOP•5w ago

2 threads trying to edit data at once while having the same version in its command one has to fail

dan1st•5w ago

yeah but can't it happen that they run after each other resulting it to fail with your if?

ayylmao123xddOP•5w ago

it can happen because if both requests send version 1 and entity in db is 1 then they both pass through the if statement

dan1st•5w ago

well it might happen that they go into the if the test doesn't verify that JPA optimistic locking is the thing that fails

ayylmao123xddOP•5w ago

yes but its quite unlikely

dan1st•5w ago

well do you not want to test that unlikely case where JPA does that? actually I forgot a mock spec

ayylmao123xddOP•5w ago

oh i also made another test where it sends a wrong version it throws an error correctly

JavaBot•4w ago

💤 Post marked as dormant

This post has been inactive for over 300 minutes, thus, it has been archived. If your question was not answered yet, feel free to re-open this post or create a new one. In case your post is not getting any attention, you can try to use /help ping. Warning: abusing this will result in moderative actions taken against you.

Java Community | Help. Code. Learn.Join

With more than 20,000 members, join one of the biggest and most active Java Communities on Discord! ☕

25KMembers

View on Discord

Want results from more Discord servers?

Add your server