Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Jobs are scheduled with the same parameters even though different parameters are provided #335

Closed
JanHolger opened this issue Jan 23, 2022 · 14 comments
Assignees
Labels
Awaiting feedback bug Something isn't working
Milestone

Comments

@JanHolger
Copy link

Are you running the latest version of JobRunr?
Yes

Describe the bug
We have a Job class with a run method that takes a uuid as it's parameter. We then enqueue the jobs using the following call:

BackgroundJob.enqueue(() -> new OurJob().run(id));

This works perfectly fine for the first run in the process but if we call this multiple times with different id's it creates the jobs with the first id over and over. We've validated that our id's are indeed different each time so there has to be a problem in JobRunr or the way we are using it.

As we are not running in Spring, we are providing a pretty simple activator but from what I understand this doesn't really matter as our job class doesn't contain any fields and the dashboard also shows the wrong id so they are already getting scheduled with the wrong parameters.

new JobActivator() {
    public <T> T activateJob(Class<T> aClass) {
        try {
            return aClass.newInstance();
        } catch (InstantiationException | IllegalAccessException e) {
            e.printStackTrace();
        }
        return null;
    }
}

Environment
I'm using JobRunr version: 4.0.6
I'm running on JRE / JDK (e.g. OpenJDK 1.8.0_292): OpenJDK 1.8.0_302
I'm using the following StorageProvider (e.g. Oracle / MongoDB / ...): MariaDB

How to Reproduce

public class OurJob {
  public void run(UUID id) {
    System.out.println(id);
  }
}
BackgroundJob.enqueue(() -> new OurJob().run(UUID.randomUUID()));
BackgroundJob.enqueue(() -> new OurJob().run(UUID.randomUUID()));

Expected behavior
The job should get scheduled with 2 different id's.

@rdehuyss
Copy link
Contributor

I'm quite sure this is because of the jobdetails caching which is too aggressive.

In the configuration, try using .useJobDetailsGenerator(new JobDetailsAsmGenerator())

Can you report back if this solves your problem?

@JanHolger
Copy link
Author

This indeed seems to have solved the issue. If this is expected behavior it might be good idea to add a hint to the docs. Thank you really much for the fast response and let me know if we can help finding the issue with further tests!

@rdehuyss
Copy link
Contributor

No worries, happy it is working again and glad that I can help a sponsor!

You could enable caching probably again by extracting the UUID generation outside the enqueue method. Let me know how that works out.

@JanHolger
Copy link
Author

In the actual setup the uuid belongs to a database model and is already generated outside the lambda. Before the enqueue call i'm assigning it to a local variable to make it effectively final.

1 similar comment
@JanHolger
Copy link
Author

In the actual setup the uuid belongs to a database model and is already generated outside the lambda. Before the enqueue call i'm assigning it to a local variable to make it effectively final.

@rdehuyss
Copy link
Contributor

Could you share that piece of code?

@JanHolger
Copy link
Author

In the actual setup the uuid belongs to a database model and is already generated outside the lambda. Before the enqueue call i'm assigning it to a local variable to make it effectively final.

@JanHolger
Copy link
Author

Unfortunately I can't share the exact code but that's basically it:

model.setSomething(123);
model.save(); // this generates the uuid and saves it to the database

// ... some more checks and stuff

UUID id = model.getId();
BackgroundJob.enqueue(() -> new OurJob().run(id));

The save method is coming from an abstract class that the model class inherits. This is coming our own framework's orm (https://github.com/JavaWebStack/orm) in case thats important.

@rdehuyss
Copy link
Contributor

rdehuyss commented Jan 26, 2022

Hi @JanHolger , I'll need some help from you to reproduce this. I've tried to do so, see https://github.com/jobrunr/jobrunr/blob/bugfix/github-issue335/core/src/test/java/org/jobrunr/scheduling/BackgroundJobByJobLambdaTest.java#L485-L492.

Yet, I see 2 different UUID's logged and I've almost copied the code from you above. Can you shed some light on what you guys are doing?

@rdehuyss rdehuyss self-assigned this Jan 26, 2022
@rdehuyss rdehuyss added bug Something isn't working Awaiting feedback labels Jan 26, 2022
@rdehuyss rdehuyss modified the milestone: 5.0.0 Jan 26, 2022
@JanHolger
Copy link
Author

Sorry that I took so long to reply. Your test is generating the UUID in the lambda so it will of course be different. Our code is more like the one that I shared in my last comment. I've copied our code again and stripped and replaced all names because it's internal code. The actual controller method looks something like this but there are a lot more checks and multiple related entities involved:

@Post
public Response create(Exchange exchange, SomeCreateRequest request) {
    
    // Searching for related objects in the database and returning a 4xx Response in case they are invalid/missing
    RelatedModel relatedObject = Repo.get(RelatedModel.class).get(request.getRelatedId());
    if(relatedObject == null)
        return Response.error(404, "relatedModel.notFound");

    // Creating the instance of our entity
    SomeModel someObject = new SomeModel()
            .setName(request.getName());

    someObject.save(); // This generates the id and persists the object to the database
    
    // Assign the related entity to the newly created one
    relatedObject.setSomeId(someObject.getId()).save();

    // Create a field to store the uuid in because serializing the entire entity object would be pointless
    UUID id = someObject.getId();

    // Enqueue a job for the entity
    BackgroundJob.enqueue(() -> new SomeJob().run(id));

    return Response.resource(SomeResource.class, someObject); // Return the entity mapped to SomeResource

}

I can try to find some time tomorrow to reproduce it in an example application that I can share.

@rdehuyss
Copy link
Contributor

rdehuyss commented Jan 29, 2022

If you could setup an example project that reproduces this, that would be great! I also have generated the uuid outside the lambda and that worked also. It would be great if you could setup a repo that reproduces it.

@keldkemp
Copy link

Hello!

If correctly understood by the example of the author. That can be repeated in this way:

    void testGithubIssue335() {
        UUID uuid1 = UUID.randomUUID();
        System.out.println("UUID1: " + uuid1);

        UUID uuid2 = UUID.randomUUID();
        System.out.println("UUID2: "+ uuid2);

        setJob(uuid1);
        setJob(uuid2);
    }

    private void setJob(UUID uuid) {
        BackgroundJob.enqueue(() -> new TestService.GithubIssue335().run(uuid));
    }

P.S. Sorry. I understand English badly. Used google translator

@rdehuyss
Copy link
Contributor

Thanks, I was able to reproduce it.

@rdehuyss rdehuyss modified the milestones: 5.0.0, 4.0.7 Jan 30, 2022
rdehuyss added a commit that referenced this issue Jan 30, 2022
@rdehuyss
Copy link
Contributor

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting feedback bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants