Fix #55: Old queues have not been removed after a job is regenerated by JobDSL

[alexsedova]

Definition of Done for the pull request

• [ ] Travis CI build is passing (includes unit tests and static analysis)
• [ ] You submission reviewed by one of the maintainers
• [ ] New functionality covered by unit tests
• [ ] Help messages for UI updated
• [ ] Screenshots in README.md updated
• [ ] JobDSL help updated
• [ ] Groovy script for global configuration updated

Recomendations

Pull request title

• Fix #X: <issue X title> in case if you have an issue
• Update for the change introduced in #X (where X is issue or pull request) in case of no issue

Merging pull request

Please use squash merge by default unless you have a very good reason not to do so

[Andrey9kin]

@alexsedova have you considered implementing other methods defined by ItemListener interface. As far as I can see there is one called onUpdated which is probably the one called by JobDSL. My point is what if we implement the rest of the methods like onUpdated, onRenamed, onCopied and onLoaded? Will that solve the problem?

http://javadoc.jenkins.io/hudson/model/listeners/ItemListener.html

[alexsedova]

@Andrey9kin The problem is those methods are not called, and ItemListener seems not listening then JobDSL generates a job. There is no option to call onCreate or other functions from ItemListner during the job generation

[Andrey9kin]

@alexsedova are you sure about that? Did you test it? I suspect that either onUpdated or onLoaded should be called.

[alexsedova]

I debugged the code. When you manipulate manually ItemListner will call onCreate or onUpdate. But when seed job generates a job nothing happens.

[Andrey9kin]

@alexsedova can you test onLoaded for me? My thinking is that JobDSL creates xml files and it have to ask Jenkins to load them. So something should happen and onLoaded seems to be a good candidate

[alexsedova]

The point is to find a trigger to load. The trigger is not stopped before started that is why we have all these troubles. onLoaded() itself is useless, I need to implement this but there are no arguments I can use to find the trigger. Look at the onDeleted() there is item argument we can use to find a trigger to stop.

[Andrey9kin]

@alexsedova I did some reading and here is what people say - generally, you want to create one connection and then many channels inside that connection. Every connection is TCP connection which is expensive to handle, and every channel is a virtual pipe within TCP connection. So in this way we are saving resources.

With current implementation of broker and RabbitMQConnection, we will create a new connection every time someone asks for a channel

    public Channel createChannel() throws IOException, TimeoutException {
        if(channel != null) {
            return channel;
        }
        channel = factory.newConnection().createChannel();
        return channel;
    }

So what we should probably do is to have a static field connection inside RabbitMQConnection and create channels within this connection. If the job being recreated then, we will have a hanging channel, not a connection. In this case, we do not need RabbitMQConnectionHolder because RabbitMQConnection will take care of keeping a connection and sharing it between threads. This is only a part of the solution. The second part is to have a map with channels associated with jobs and close them when needed. So more or less as you propose