Trigger problem when job config reloaded

66 views
Skip to first unread message

gal...@redhat.com

unread,
Dec 4, 2014, 12:20:34 PM12/4/14
to jenkin...@googlegroups.com
I have a plugin that extends the Trigger class. It creates a JMS connection and consumes messages from a queue, scheduling a job if a particular message is received.

Both the connection and consumer are declared as private, transient variables in the class. The connection and consumer are initialized in the start method and destroyed in the stop method.

This works fine when the job configuration is updated via the UI.

However, I run into problems when the job configuration is 1) reloaded from disk, 2) updated via the jenkins API (HTTP POST method), or 3) updated with jenkins job builder. In these cases it seems that the stop method is never called, which causes me to get a connection exists errors in the start method (which does get called).

I've made my start method smart enough to check to see if the private variables are non-null. In these cases, however, they are null - but the connection still exists. It seems as though an entirely new trigger object has been created without destroying the old one, and I can't figure out how to reference the old one so I can clean up.

I'm pretty sure I'm missing something basic here, but I don't know what. This seems like a problem others would have hit, but I haven't been able to find anything similar.

Can anyone help?

Thanks,

-- Greg 

Daniel Beck

unread,
Dec 4, 2014, 1:17:41 PM12/4/14
to jenkin...@googlegroups.com
Looks a lot like the problem Gerrit Trigger had (has?).

https://issues.jenkins-ci.org/browse/JENKINS-23152
> --
> You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/399f4fb0-217b-4d0a-94cf-de8573fb0424%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Greg Allen

unread,
Dec 4, 2014, 2:11:05 PM12/4/14
to jenkin...@googlegroups.com

On 12/04/2014 01:14 PM, Daniel Beck wrote:
Looks a lot like the problem Gerrit Trigger had (has?).

https://issues.jenkins-ci.org/browse/JENKINS-23152

Yes, I saw that bug. And specifically this comment:

> @rsandell as noted in the PR comments, while expanding the calls made to Trigger.stop
> from core might fix the bug as produced by Reload Configuration from Disk, other problematic
> cases will likely remain—folder moves, etc.—and you will be tracking them all down for a
> long time. The root problem is that GerritTrigger and related classes hold references to
> AbstractProject yet are used in global static state, which is inherently risky and a design
> problem: items are intended by held only from their ItemGroup (ultimately via the Jenkins
> singleton). I believe my PR corrects that problem, or at least gives the general idea, but
> the plugin tests would need extensive edits to keep up.

This was back in May, and I can't tell if this change actually would fix my problem - it appears the fix
was to GerritTrigger and not the core so I suspect not.

-- Greg

Jesse Glick

unread,
Dec 4, 2014, 2:40:26 PM12/4/14
to Jenkins Dev
On Thu, Dec 4, 2014 at 12:20 PM, <gal...@redhat.com> wrote:
> when the job configuration is 1) reloaded from
> disk, 2) updated via the jenkins API (HTTP POST method), or 3) updated with
> jenkins job builder. In these cases it seems that the stop method is never
> called

Yes, a core bug, probably not filed. Currently Trigger.stop is called
only from the UI-driven configure method. Perhaps the same for .start,
not sure.

Feel free to file the bug, especially if you have concrete cases that
reproduce it. Fixing that bug would help with certain cases of
JENKINS-23152, though probably not all, hence my comment; if you file
it, link to JENKINS-23152.

> It seems as though an entirely new trigger object
> has been created without destroying the old one

Correct. Objects like builders and triggers are freely recreated from
JSON form submissions, disk deserialization, and so on, and so should
generally not have any mutable state.

> I can't figure out how to reference the old one so I can clean up.

I would try using a WeakHashMap keyed off the job to keep track of
what connections you have outstanding. Just take care that the value
(whatever that is—a connection, perhaps) does not strongly refer to
the key, directly or indirectly, or the entry will never be collected.
And remember that there is no absolute guarantee that Trigger.stop
will ever be called, since the whole project could simply be deleted,
for example.

Robert Sandell

unread,
Dec 5, 2014, 4:54:21 AM12/5/14
to jenkin...@googlegroups.com
Yes,
this is what the Gerrit Trigger has had problems with for a long time, not many triggers are referencing resources like this and combined with how few are actually configuring via different means than the UI and so it's not showing that often.

Trigger.start is at least, as you say called when a config.xml post is done.


I haven't had time to dig into how to potentially solve calling stop on the old instance in this scenario though. So there are still potential issues with rename and move.

/B

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--
Robert Sandell
Software Engineer
CloudBees Inc.

Stephen Connolly

unread,
Dec 6, 2014, 3:43:39 AM12/6/14
to jenkin...@googlegroups.com


On Thursday, December 4, 2014, Jesse Glick <jgl...@cloudbees.com> wrote:
On Thu, Dec 4, 2014 at 12:20 PM,  <gal...@redhat.com> wrote:
> when the job configuration is 1) reloaded from
> disk, 2) updated via the jenkins API (HTTP POST method), or 3) updated with
> jenkins job builder. In these cases it seems that the stop method is never
> called

Yes, a core bug, probably not filed. Currently Trigger.stop is called
only from the UI-driven configure method. Perhaps the same for .start,
not sure.

Feel free to file the bug, especially if you have concrete cases that
reproduce it. Fixing that bug would help with certain cases of
JENKINS-23152, though probably not all, hence my comment; if you file
it, link to JENKINS-23152.

> It seems as though an entirely new trigger object
> has been created without destroying the old one

Correct. Objects like builders and triggers are freely recreated from
JSON form submissions, disk deserialization, and so on, and so should
generally not have any mutable state.

This is where the Node vs Computer parallel object trees come in.

I would have an Extension that holds these "persistent mutable" objects in say a Map<String,Connection>

Then the Trigger uses that extension to look up connections for items (the key being the item path). 

The extension would have an item listener to catch renames and deletes (watch your synchronisation there, you will need to use good locking patterns... Ignore the crazy over-use of volatile demonstrated in the jenkins code... Lots of bugs come from how jenkins incorrectly handles multi-threading) 

The extension would also have a periodic work that walks the map purging (and stopping) and connections for items that no-longer exist

IOW the extension is responsible for creating, maintaining, and stopping the connections. The trigger is responsible for linking the items to the connections managed by the extension.

HTH

> I can't figure out how to reference the old one so I can clean up.

I would try using a WeakHashMap keyed off the job to keep track of
what connections you have outstanding. Just take care that the value
(whatever that is—a connection, perhaps) does not strongly refer to
the key, directly or indirectly, or the entry will never be collected.
And remember that there is no absolute guarantee that Trigger.stop
will ever be called, since the whole project could simply be deleted,
for example.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr2tGHNkVURwCj6Kn1V%2BGQmo3prtMCkYrTWFV8kDMBSxAw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


--
Sent from my phone
Reply all
Reply to author
Forward
0 new messages