groovy-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ion Alberdi (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (GROOVY-8097) Add an argument to set the resolution cache path in @Grab
Date Tue, 14 Mar 2017 08:30:41 GMT

    [ https://issues.apache.org/jira/browse/GROOVY-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15907001#comment-15907001
] 

Ion Alberdi edited comment on GROOVY-8097 at 3/14/17 8:30 AM:
--------------------------------------------------------------

[~jexler] Please find in https://github.com/yetanotherion/code_snippets/blob/master/groovy/groovy_8097/grab.groovy/(https://github.com/yetanotherion/code_snippets/blob/master/groovy/groovy_8097/grapeConfig.xml,
to put in $HOME/.groovy) the test we used to reproduce/validate the concurrent access-es to
repository/resolution cache dirs. This test launches 200 threads
that each download different versions of org.jenkins-ci.main:jenkins-core and org.codehaus.groovy:groovy-all.
As said earlier,
- the global lock strategy will be slower than the different resolution cache dir solution
(as we'll get a lock even if different artifacts should be downloaded)
- the solution we're looking for, should work for both *same jvm* and *multiple jvm scenarios*
(launching two groovy grab.groovy should work too, even though the current state of grab.groovy
does not handle it (a prefix should be added so that the two jvm compute different paths for
the resolution cache dir of the 200 threads in the same jvm)).

For these reasons the solution proposed by [~paulk] seems more efficient/maintanable  (Regarding
maintainability: "The call would have to be made at least once per VM resp. per loaded Grape.class,
preferably before you do any grabs. Instead of passing Date.class as a lock, you can pass
any class in the JDK, it just has to be something that is not loaded multiple times in the
VMs.", seems harder to guess than "one resolution cache dir per concurrent job").

[~paulk] sorry for having made you repeat something you already said in the first answer of
the post. 

If you all agree, the first version of the PR will 
- follow [~paulk] suggestion and among others, 
- "Grape will switch from a singleton to a map of <string: resolutionCacheDir, GrapeIvy.class:
grapeIvy,> each grapeIvy pointing to a different resolutionCacheDir."





was (Author: yetanotherion):
[~jexler] Please find in grab.groovy/(grapeConfig.xml, to put in $HOME/.groovy) the test we
used to reproduce/validate the concurrent access-es to repository/resolution cache dirs. This
test launches 200 threads
that each download different versions of org.jenkins-ci.main:jenkins-core and org.codehaus.groovy:groovy-all.
As said earlier,
- the global lock strategy will be slower than the different resolution cache dir solution
(as we'll get a lock even if different artifacts should be downloaded)
- the solution we're looking for, should work for both *same jvm* and *multiple jvm scenarios*
(launching two groovy grab.groovy should work too, even though the current state of grab.groovy
does not handle it (a prefix should be added so that the two jvm compute different paths for
the resolution cache dir of the 200 threads in the same jvm)).

For these reasons the solution proposed by [~paulk] seems more efficient/maintanable  (Regarding
maintainability: "The call would have to be made at least once per VM resp. per loaded Grape.class,
preferably before you do any grabs. Instead of passing Date.class as a lock, you can pass
any class in the JDK, it just has to be something that is not loaded multiple times in the
VMs.", seems harder to guess than "one resolution cache dir per concurrent job").

[~paulk] sorry for having made you repeat something you already said in the first answer of
the post. 

If you all agree, the first version of the PR will 
- follow [~paulk] suggestion and among others, 
- "Grape will switch from a singleton to a map of <string: resolutionCacheDir, GrapeIvy.class:
grapeIvy,> each grapeIvy pointing to a different resolutionCacheDir."




> Add an argument to set the resolution cache path in @Grab
> ---------------------------------------------------------
>
>                 Key: GROOVY-8097
>                 URL: https://issues.apache.org/jira/browse/GROOVY-8097
>             Project: Groovy
>          Issue Type: New Feature
>          Components: Grape
>    Affects Versions: 2.4.8
>            Reporter: Ion Alberdi
>            Priority: Minor
>         Attachments: grab.groovy, grapeConfig.xml
>
>
> Ivy does not support concurrent access to its resolution cache 
> https://issues.apache.org/jira/browse/IVY-654
> Grape relies on Ivy. For this reason, Grape cannot support concurrent access to its resolution
cache neither.
> When using the @Grab annotation in jenkins groovyCommand or systemGroovyCommand, the
related code is vulnerable to race conditions. When the race condition appears in a systemGroovyCommand,
we have no choice but to reboot jenkins as all consecutive calls to @Grab fail.
> Among the two solutions we tried: 
> - Protect the calls to grab with a lock similar to ivy's "artifact-lock-nio" strategy.
Works but slow.
> - Set Ivy's lock on the repository cache and setup Grab to use a different cache resolution
cache for each concurrent jobs. The following code permits to fix a test we did to reproduce
the race condition.
> {code}
>     static IvySettings createIvySettings(String resolutionPath, boolean dumpSettings)
{
>         // Copy/Paste/Purged from GrapeIvy.groovy
>         IvySettings settings = new IvySettings()
>         settings.load(new File(GROOVY_HOME, "grapeConfig.xml"))
>         // set up the cache dirs
>         settings.defaultCache = new File(GRAPES_HOME)
>         settings.setVariable("ivy.default.configuration.m2compatible", "true")
>         settings.setDefaultResolutionCacheBasedir(resolutionPath)
>         return settings
>     }
>     static GrapeIvy ivyWithCustomResolutionPath(String resolutionPath) {
>         Class<?> grapeIvyClass = Class.forName("groovy.grape.GrapeIvy");
>         Object instance = grapeIvyClass.newInstance()
>         Field field = grapeIvyClass.getDeclaredField("ivyInstance");
>         field.setAccessible(true);
>         field.set(instance, Ivy.newInstance(createIvySettings(resolutionPath)));
>         return ((GrapeIvy)instance)
>     }
> {code}
> We'd like to propose to add an additional argument to Grab to setup Ivy's resolution
cache directory.
> Note that this solution seems to have been adopted by these users too
> https://rbcommons.com/s/twitter/r/3436/
> Would you agree on such a feature ? We'd be glad to propose a PR.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message