jmxtrans icon indicating copy to clipboard operation
jmxtrans copied to clipboard

Add UUID suffix to trigger name

Open DanilSerd opened this issue 7 years ago • 4 comments

We saw the following error in our logs:

[20 Apr 2018 06:10:15] [WrapperSimpleAppMain] 1004   ERROR (com.googlecode.jmxtrans.JmxTransformer:190) - Error scheduling job for server: Server(pid=null, host=10.x.y.z, port=9016, url=service:jmx:rmi:///jndi/rmi://10.x.y.z:9016/jmxrmi, cronExpression=null, numQueryThreads=8)
com.googlecode.jmxtrans.exceptions.LifecycleException: Error scheduling job for server: Server(pid=null, host=10.x.y.z, port=9016, url=service:jmx:rmi:///jndi/rmi://10.x.y.z:9016/jmxrmi, cronExpression=null, numQueryThreads=8)
        at com.googlecode.jmxtrans.JmxTransformer.processServersIntoJobs(JmxTransformer.java:375)
        at com.googlecode.jmxtrans.JmxTransformer.startupSystem(JmxTransformer.java:318)
        at com.googlecode.jmxtrans.JmxTransformer.start(JmxTransformer.java:187)
        at com.googlecode.jmxtrans.JmxTransformer.doMain(JmxTransformer.java:157)
        at com.googlecode.jmxtrans.JmxTransformer.main(JmxTransformer.java:138)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:606)
        at org.tanukisoftware.wrapper.WrapperSimpleApp.run(WrapperSimpleApp.java:240)
        at java.lang.Thread.run(Thread.java:745)
Caused by: org.quartz.ObjectAlreadyExistsException: Unable to store Trigger with name: '10.x.y.z:9016-1524204615162' and group: 'DEFAULT', because one already exists with this identification.
        at org.quartz.simpl.RAMJobStore.storeTrigger(RAMJobStore.java:314)
        at org.quartz.simpl.RAMJobStore.storeJobAndTrigger(RAMJobStore.java:194)
        at org.quartz.core.QuartzScheduler.scheduleJob(QuartzScheduler.java:822)
        at org.quartz.impl.StdScheduler.scheduleJob(StdScheduler.java:243)
        at com.googlecode.jmxtrans.JmxTransformer.scheduleJob(JmxTransformer.java:410)
        at com.googlecode.jmxtrans.JmxTransformer.processServersIntoJobs(JmxTransformer.java:371)
        ... 10 more

From what we could see it seems that just the nanoTime() is not enough to uniquely name the trigger

DanilSerd avatar Apr 20 '18 09:04 DanilSerd

:x: Build jmxtrans 189 failed (commit https://github.com/jmxtrans/jmxtrans/commit/9584dcc338 by @)

AppVeyorBot avatar Jun 12 '18 10:06 AppVeyorBot

:white_check_mark: Build jmxtrans 190 completed (commit https://github.com/jmxtrans/jmxtrans/commit/a188e91f75 by @)

AppVeyorBot avatar Jun 12 '18 10:06 AppVeyorBot

@DanilSerd Looking at the overall code, I noticed this piece of code just above:

	private void scheduleJob(Server server) throws ParseException, SchedulerException {
		String name = server.getHost() + ":" + server.getPort() + "-" + System.nanoTime() + "-" + RandomStringUtils.randomNumeric(10);

This is basically what you're trying to do, are you? Then can we reuse either either the generated name or the name generator?

gquintana avatar Jul 01 '18 21:07 gquintana

A general comment here: The right approach here should not be to try to generate a unique name for this trigger, but should be to get rid of Quartz and use a much simpler ScheduledExecutorService.scheduleAtFixedRate(). We might keep Quartz to support more generic cron expressions, but honestly, I think we should drop support for cron expressions. Given the use case of jmxtrans, I doubt that crons see much use.

gehel avatar Jul 04 '18 07:07 gehel