lockable-resources-plugin icon indicating copy to clipboard operation
lockable-resources-plugin copied to clipboard

Proposal for issue #128: Ephemeral labels

Open sleepy-manul opened this issue 5 years ago • 10 comments

The original motivation for this PR was that I needed fine-grained control over the parallelism in my declarative pipeline. For instance, I needed the ability to allow two parallel jobs within the same Kubernetes build pod, but not three (since that would need too much RAM and CPU, thus crashing my pod).

Since lockable-resources-plugin already makes this possible in the case where I can define the labels beforehand, I extended the functionality of auto-generating resources to auto-generating labels with a given quantity of resources. The resource names will be auto-generated in the form "LABEL-0000-some-UUID-0000" in order to avoid conflicts with other resources.

Another motivation was that I wanted to change as few code lines as possible and not break existing behaviour. This is why a user of ephemeral labels must explicitly ask for auto-creation. This is different from single resources, but it should prevent breaking existing pipelines that use lockable-resources.

  • Introduce a new lockStep-Parameter "createLabelWithQuantity".
  • Make test lockWithLabelConcurrent quicker and ensure that all spawned jobs are checked for success
  • Mark test lockWithLabelConcurrent as not working on Windows (seems to work fine on Unix)
  • Splits the code for unlocking/freeing resources and deleting obsolete ephemeral resources into two methods (see the comment in unlockNames() for details()
  • Be more specific inside the code on when createResource() is allowed to auto-create ephemeral resources

sleepy-manul avatar May 08 '20 12:05 sleepy-manul

From a cursory look, this seems quite sensible. I'll try to review more thoroughly in the coming days.

TobiX avatar May 08 '20 15:05 TobiX

@TobiX I see in your CI environment that there are error messages thrown by the Maven Enforcer Plugin, but I could not reproduce the problem in my build containers. Do you have any hints that can help me diagnose it?

sleepy-manul avatar May 08 '20 16:05 sleepy-manul

Yeah, that's the standard Jenkins CI checking for compatibility with newer Jenkins LTS versions (runs maven with -Djenkins.version=2.222.3) and that makes the enforcer plugin cranky. I'll try to fix it on master so pull requests have it easier...

TobiX avatar May 08 '20 17:05 TobiX

@stupiddog1979 Thanks a bunch for making this PR!

@TobiX @amuniz Great job maintaining this plugin!

This change looks good to me, however I have a feedback on few things here, have tested couple of scenarios locally by installing the incremental version.

  • Can the quantity be specified at resource level / at global configuration section so that random pipeline won't try to set the limits or doesn't mutually try to set that limit
  • First pipeline wins when two or more pipeline runs are trying to set the quantity, thats good, this could be problem when these pipelines are trying to set the different quantity for same label.
  • Not sure if this would be a that big concern, when the quantity is so big like even 100+, it is creating that many resources and when I look at the lockable resources page there are so many lines to work on just in case if I have to manually release or reserve all of those. Also probably takes more memory?

Example below if Pipeline2 runs first all the Pipeline1 runs has to wait until all of the pipeline 2 runs are completed (including those are in queue)

Pipeline1:

lock(createLabelWithQuantity: 20, quantity: 2, label: 'testing') {
    sleep 100
}

Pipeline2:

lock(createLabelWithQuantity: 2, quantity: 2, label: 'testing') {
    sleep 1000
}

One of the screenshots from testing: Screen Shot 2020-06-05 at 7 43 43 AM

nrayapati avatar Jun 05 '20 12:06 nrayapati

@stupiddog1979 @TobiX @amuniz Any update on this PR? Appreciate if we can have this functionality released soon. Thank you!

nrayapati avatar Jun 16 '20 18:06 nrayapati

Second that, I'd also like to see this pushed. Any help with making that happen would be greatly appreciated!

ilyache avatar Jun 29 '20 16:06 ilyache

Any update on this proposal would be greatly appreciated. Thank you for the good work on this plugin!

nrayapati avatar Jul 09 '20 18:07 nrayapati

@sleepy-manul, are you still working on this to remove the conflicts (probably needs a rebase)? We would be interested in this fix, since this solves a very specific problem that would enable real throttling of parallel stages. @TobiX, what would be required to get this merged aside from getting rid of the conflicts (rebasing)? Thanks in advance!

dsteiert avatar Sep 15 '22 17:09 dsteiert

I think this plugin needs a new maintainer to go through the PR queue. The plugin is up for adoption and anyone can adopt the plugin and become a maintainer to merge and release PRs like this one.

basil avatar Sep 15 '22 17:09 basil

Yeah... on my side got overwhelmed with real life and other projects. Keep hoping to get back to such stalled older commitments :\

jimklimov avatar Sep 30 '22 22:09 jimklimov

Please make it happen

igor-sidorov avatar Nov 03 '23 08:11 igor-sidorov