quarkus-quartz duplicating clustered jobs

[brunobat]

Describe the bug
When more than one job is configured with quarkus-quartz in a clustered environment we are seeing a duplication on the registered db jobs.

The jobs are registred like this:

 @Scheduled(cron = "{full.qualified.classA.cron}")
  void schedule()
...
 @Scheduled(cron = "{full.qualified.classB.cron}")
 void executeJob()

Then, 2 instances of the service are started but the problem also happens with just a single instance.

On the the qrtz_triggers.trigger_name column we will see new jobs like this:

0_full.qualified.classA_ScheduledInvoker_executeJob_952519a823738725fb11b965dd917e07b656fb4d
1_full.qualified.classB_ScheduledInvoker_schedule_dce0524c3f1de653f83719900424446459b7c471

If we stop the service and start it again enough times, at some point we will see:

0_full.qualified.classA_ScheduledInvoker_executeJob_952519a823738725fb11b965dd917e07b656fb4d
1_full.qualified.classB_ScheduledInvoker_schedule_dce0524c3f1de653f83719900424446459b7c471
0_full.qualified.classB_ScheduledInvoker_schedule_dce0524c3f1de653f83719900424446459b7c471
0_full.qualified.classA_ScheduledInvoker_executeJob_952519a823738725fb11b965dd917e07b656fb4d

As you can see, the first digit changes, thus dupluting the jobs.

After inspecting the code here:
https://github.com/quarkusio/quarkus/blob/ffc1c5b9423a47cf441d0f860f82312dbb266658/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzScheduler.java#L97

We can see that the 1st digit is an increment. We can asusme that the job order is not always the same and this increment later causes the duplication.

Job creation is not deterministic.

We are using quarkus 1.2.1.Final, but the above link is from the latest and seems to have the issue as well.

Expected behavior
Clustered jobs should not be duplicated.

Actual behavior
Clustered jobs are being duplicated.

To Reproduce
see description

Configuration

full.qualified.classA.cron=0 0 * * * ?
full.qualified.classB.cron=0 0 * * * ?

Screenshots
no need

Environment (please complete the following information):

• PostgreSQL 11.2-alpine
• Apache Maven 3.6.1 (d66c9c0b3152b2e69ee9bac180bb8fcc8e6af555; 2019-04-04T20:00:29+01:00)
  Maven home: /Users/joao.rafael/Software/apache-maven-latest
  Java version: 14, vendor: AdoptOpenJDK, runtime: /Library/Java/JavaVirtualMachines/adoptopenjdk-14.jdk/Contents/Home
  Default locale: en_US, platform encoding: UTF-8
  OS name: "mac os x", version: "10.14.6", arch: "x86_64", family: "mac"
  
  
  
  • This also happens with Java 11.
  
  

Additional context
none

[gsmet]

/cc @machi1990 @mkouba

[brunobat]

I wonder if we could, in the future, configure the job name on the annotation, That way, we wouldn't need to generate a name.

[machi1990]

@brunobat thanks for reporting the issue and the investigation.

Quick question:

If we stop the service and start it again enough times, at some point we will see:

Do you rebuild the application and start it again? Or simply a restart of the already built one?

[mkouba]

What do you mean by "2 instances are started"?

Anyway, I do understand that non-deterministic job names are a problem for clustered jobs - that's a leftover from the times when clustered jobs were not supported.

[gsmet]

I think if you start two instances of your Quarkus app and jobs are created in a different order, you end up with 2 jobs started instead of one.

[jp-rafael]

Do you rebuild the application and start it again? Or simply a restart of the already built one?

@machi1990, a restart of the already built one also causes this problem.

[jp-rafael]

What do you mean by "2 instances are started"?

@mkouba, it means two instances of the same application executing in different ports. But this also happens with a single instance.

[machi1990]

Do you rebuild the application and start it again? Or simply a restart of the already built one?

@machi1990, a restart of the already built one also causes this problem.

I have tried this across several runs but was not able to reproduce the problem however by building the application each time I was able to reproduce the issue.

[mkouba]

I came to the same conclusion - the job names are the same _unless_ you rebuild the app. The names are generated every time the app starts but since the list of scheduled methods is hardcoded in a build step the result should be identical.

I believe that we should remove the sequence from the name completely. AFAIK the original intent was to support multiple scheduled methods of the same name. But since the name of the generated invoker class already contains a hash built from the method signature there is no need to include the sequence at all.

@machi1990 ^ WDYT

[jp-rafael]

I have tried this across several runs but was not able to reproduce the problem however by building the application each time I was able to reproduce the issue.

I came to the same conclusion - the job names are the same unless you rebuild the app.

My apologies for the misleading information. I was not aware that mvn quarkus:dev would cause an application build.

[machi1990]

I came to the same conclusion - the job names are the same _unless_ you rebuild the app. The names are generated every time the app starts but since the list of scheduled methods is hardcoded in a build step the result should be identical.

Yes, this was my conclusion too.

I believe that we should remove the sequence from the name completely. AFAIK the original intent was to support multiple scheduled methods of the same name. But since the name of the generated invoker class already contains a hash built from the method signature there is no need to include the sequence at all.

@machi1990 ^ WDYT

I think we should still keep the sequence as we generate an invoker class for a method and since the @Scheduled annotation (the actual job) can be repeated, we need the sequence in order to support it (repeated schedules) and generate a unique id for each one of them.

i.e without the sequence, this will not work

@Scheduled(every="1s")
@Scheduled(every="10s")
public void job() {}

unless you create two methods for it.

[mkouba]

I think we should still keep the sequence as we generated an invoker class for a method and since the @Scheduled annotation (the actual job) can be repeated

Very good point! I completely forgot about this (even though I came with the idea of repeatable @Scheduled ;-).