Skip to content

Conversation

@michael-o
Copy link
Member

This closes #68

@michael-o
Copy link
Member Author

michael-o commented Aug 18, 2020

@dantran Please try this on your huge multimodule project. This should perform way better than global locking.
@khmarbaise I remember you having huge projects, can you please try this and report back?

@philippe-granet
Copy link

On unix, redis default configuration could be with Unix socket? It will be more
performant than using network?

@michael-o
Copy link
Member Author

@philippe-granet While you are correct, of course. I can't provide it. Please read: redisson/redisson#773 (comment). Redisson's code has been stupidly nailed to host:port. Many layers to be changed to make it work with UDS and TCP.

@philippe-granet
Copy link

I think you can activate an unix socket if servers binded to loopback interface (127.0.0.1 ?). It requires netty-transport-native-epoll lib in classpath. See Config.setTransportMode and https://github.com/redisson/redisson/blob/master/redisson/src/main/java/org/redisson/config/TransportMode.java

@michael-o
Copy link
Member Author

michael-o commented Aug 18, 2020

This will not and cannot work. You need to provide the bind address of the Unix socket. Read the Redisson code.

@dantran
Copy link

dantran commented Aug 19, 2020

@michael-o I built your branch (1.5.1-SNAPSHOT), and replace it at my maven 3.6.3( build from source via mvn clean install -DresolverVersion=1.5.1-SNAPSHOT -Drat.skip=true). The result is better but still slow

- Clean build no local repo - 1 hour ( no test)
- with local repo cache  2.5 min
- Clean build no local repo + original maven 3.6.3 - 6 min

The serialization of dependencies is the culprit

@michael-o
Copy link
Member Author

michael-o commented Aug 19, 2020

@dantran Thank you very much for testing. I assume that your project is not publically available. I will try to make some assumptions if you cannot share the trace logs:

  • What is the degree of parallelism you use?
  • Do your modules have a lot of dependencies?
  • Do you have many overlapping dependencies?

There could be two explanations:

  • Many many dependencies cause a lot of requests to Redis. Rountrips add time.
  • Overlapping dependencies makes the entire build thread to wait. Consider the following example: You need 100 locks and one single artifact is already locked. You need to wait n seconds. Obviously the rest cannot continue. This is an issue I (guess) cannot solve. You properly nailed it: serialization.

Do you think you could provide an obfuscated log? I'd like to see what is causing the delay. Here is the log config.
One could consider to cache the Redisson locks globally, but I don't know whether this is a good idea at all. Nor do I want to make it any more complex than necessary.

@michael-o
Copy link
Member Author

If somebody has a large OSS project at hand, let me know. I tried Payara and Windup. The overhead was very humble: 10%.

@michael-o
Copy link
Member Author

@dantran Can you please retry by increasing aether.metadataResolver.threads and aether.connector.basic.threads? Resolver acquires in many cases more locks than it processes in parallel.

@dantran
Copy link

dantran commented Aug 22, 2020

@michael-o I gave this a another try with

struggling to see any redis/netty logging showing up at console. So my build is not using Redis at all. Still looking ...

@michael-o
Copy link
Member Author

michael-o commented Aug 23, 2020

@dantran Then something hasn't been picked up. Please verify that everything required from target/dependency is in ext/redisson. Especially javax.annotation.jar otherwise the injection will not work. Also make sure that you are on 3.7.0-SNAPSHOT. I don't know whether the Sisu version with 3.6.3 can process @Priority annotation which is imperative here.

Copy link

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very interesting work.

Are you able to add some test case?

@michael-o
Copy link
Member Author

Very interesting work.

Are you able to add some test case?

I have though about this, but two points are crucial here and was not able to properly solve them:

  • It would require Redis to be locally installed. Means that Windows cannot be tested at all. One could test for redis-cli. If this is available, Redis might run locally.
  • How do I reliably test synchronized code with an executor service? I have no idea.

@eolivelli
Copy link

Oh there is no Redis server emulator for Java.

Btw I was also thinking about a simple smoke tests that connects to Redis and covers the code, this way we can see that at least it is working in a simple scenario.

Copy link

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@michael-o
Copy link
Member Author

Oh there is no Redis server emulator for Java.

Btw I was also thinking about a simple smoke tests that connects to Redis and covers the code, this way we can see that at least it is working in a simple scenario.

I'll need to think about this, but provide in a subsequent PR. Already put a week of work into it.

@dantran
Copy link

dantran commented Aug 27, 2020

@michael-o i gave maven 3.7 a try to with MRESOVER-131, still dont see traces of redis. Can I assume that since I run my Redis docker locally, there is no need for me to configure the yml settings from your instructions page?

@michael-o
Copy link
Member Author

michael-o commented Aug 28, 2020

Can I assume that since I run my Redis docker locally, there is no need for me to configure the yml settings from your instructions page?

Yes, as long as you have mapped the port from the container to your local machine and Redis is availabe through localhost:6379.

1, still dont see traces of redis.

Please provide: tree $MAVEN_HOME and well as m2.conf and the logging properties for SLF4J Simple.

and lauch with -Dsisu.debug. We should find the RedissonSyncContextFactory before the default one.

@asfgit asfgit merged commit d045b4d into master Sep 11, 2020
@michael-o michael-o deleted the MRESOLVER-131 branch September 11, 2020 15:45
@philippe-granet
Copy link

@michael-o Thanks for your great work!
What do you think of these lock file implementations in Apache Lucene:
https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/store/SimpleFSLockFactory.java
https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java

Do you think that can be used as implementation of SyncContextFactory?

@michael-o
Copy link
Member Author

@philippe-granet I need to look at them. @cstamas and me tried with a file-based one and ditched it because it wasn't working. I know that Lucene's code is very very nice. If someone can profile a PR which works or various locally hosted FS systems I would love to evaluate and include. For the time being, I will not work on that because the Redis alternative is extremely easy to set up.

@jebeaudet
Copy link
Contributor

@michael-o I'm not sure where to report this but I'm struggling to make this work, my use case is to properly support concurrent build on jenkins for many projects, we often run into some concurrency problems with sharing the m2 repo across multiple jobs.

I'm using this dockerfile to generate a maven docker image for jenkins based of these instructions. Running a build with this and -Dsisu.debug will produce exceptions Caused by: java.lang.ClassNotFoundException: org.eclipse.aether.named.support.NamedLockFactorySupport. This is solved by adding the proper jar in the docker file RUN wget -P /usr/share/maven/lib/ext/redisson/ https://repo1.maven.org/maven2/org/apache/maven/resolver/maven-resolver-named-locks/1.7.1/maven-resolver-named-locks-1.7.1.jar, this is a documentation issue I guess?

After, running the build with the debug flag will produce this output. Nothing related to connecting to redis.

Output of tree $MAVEN_HOME
Output of cat m2.conf

I'm not sure what's wrong here, any ideas?

@michael-o
Copy link
Member Author

You did not upgrade to Resolver 1.7.1

@jebeaudet
Copy link
Contributor

@michael-o Actually, I'm trying to upgrade from an old version of our custom maven jenkins image that was working and I'm running into this issue. The previous configuration of our image is this. Basically, the difference is that we were using the version 1.6.1 of the maven-resolver-synccontext-redisson jar that we include in the /lib/ext/redisson folder and that's pretty much it.

I tried to include a dependency to maven-resolver-impl 1.7.1 in my projects but it didn't change anything. I also tried setting the aether.syncContext.named.factory to rwlock-redisson to no effect. Do you have a working example somewhere I could based my tests on? I'm struggling to find documentation or examples on this.

@michael-o
Copy link
Member Author

The Redisson-based approach between 1.6 and 1.7 is, unfortunately, not compatible. It has been competely moved to the named lock approach. I highly recommend to replace 1.6.x completely 1.7.x. If you need to stick to 1.6.x, follow this guide. This cannot and will not work with 1.6.x.

@jebeaudet
Copy link
Contributor

@michael-o Oh OK good to know between 1.6.x and 1.7.x. I have it running with 1.6.x and it works great for our use case(thx for that!), I was just trying to upgrade and ran into those problems.

From what I can see, in a vanilla maven 3.8.1 install, the $MAVEN_HOME/lib contains the 1.6.2 version of maven-resolver-impl. Am I right to say that using 1.7.x is not yet compatible with a released version of maven, unless you modify those jar in the /lib folder?

Thanks again for your help!

@michael-o
Copy link
Member Author

Maven 3.8.x comes with Resolver 1.6.x because Resolver 1.7 requires Java 8 which Maven 3.8.x cannot offer. You can create a custom Maven distro to upgrade Resolver. See apache/maven@fc806a2#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8 and you are good to go.

Since you are the first user responding to use Redis for Resolver. Can you provide numbers of execution time compared to before? I.e., how does synchronization affect build times for you in percent?

@michael-o
Copy link
Member Author

I would recomment to compare the tree output of 3.8.x and 4.0.x and replace Resolver JARs.

@jebeaudet
Copy link
Contributor

Yea ok it finally makes sense. If I can suggest something, this should be in the documentation to warn others. Since the 1.7.x version is the current version, it's the documentation page you end up on on apache.org and there's no mention that it's not compatible with the current and latest version of maven. Getting the documentation for the 1.6.x version is impossible going through google at least. I can suggest a PR if you think it's worth it.

As for build time, with mvn 3.8.1, redis resolver 1.6.3 and localhost redis, I have these times for a mvn compile with with an empty .m2/repo :
with redis : 04:36 min
without redis : 04:14 min

With an already populated .m2/repo (running a mvn clean before) :
with redis : 21.343s
without redis : 18.127s

To me, it's a very much acceptable tradeoff to be able to run multiple builds in parallel in jenkins by sharing the same .m2 repo!

@jebeaudet
Copy link
Contributor

I figured I'd educate myself so I built maven 3.8.1 with maven-resolver set to 1.7.0 and run the tests with mvn compile -Daether.syncContext.named.factory=rwlock-redisson for the same project
empty .m2/repo :
with redis : 4:38m
without redis : 4:48m

populated .m2/repo preceded by a mvn clean :
with redis : 19.964s
without redis : 17.679

So it seems that the differences in my other post were most likely related to I/O speed fetching all the dependencies more than the overhead by redis locks.

@michael-o
Copy link
Member Author

I have already noticed that documentation is weak. We were more focused on proper functionality. I also have founds bugs in Redisson with this new module. Please file an issue about the missing documentation.

As far as I understand your numbers, if you'd have to use seperate local repos, you would have to add at least four minutes to each build? If so, that is fantastic.

@jebeaudet
Copy link
Contributor

@michael-o Yup that's correct, reusing the same m2 repo for different builds cut the build time dramatically, that's exactly why I ended up using your plugin for secure multithreaded build on our CI :D

I've logged https://issues.apache.org/jira/browse/MRESOLVER-188 for the documentation.

@michael-o
Copy link
Member Author

michael-o commented Jul 7, 2021

@michael-o Yup that's correct, reusing the same m2 repo for different builds cut the build time dramatically, that's exactly why I ended up using your plugin for secure multithreaded build on our CI :D

That's perfect. Months of work worth going for. Can you also compare with semaphore-redisson? I wonder how this performs. Note that you must have at least Redisson 3.15.6.

I've logged https://issues.apache.org/jira/browse/MRESOLVER-188 for the documentation.

Will take care soon.

@jebeaudet
Copy link
Contributor

I'll test it out tomorrow and come back to you. Thanks again

@jebeaudet
Copy link
Contributor

with semaphores : 19.795s
without : 17.418s

This is using my custom build mvn with redisson 3.15.6 and maven-resolver-named-locks-redisson-1.7.1. Again, negligible difference on my end.

I've tried to make it a bit more interesting, I've cloned 4 times my repository and I built the project with the T1C flag and start the 4 builds at the same time. Looks like a ~5-10% impact.
mvn compile -T1C
build 1: 39.013s
build 2: 39.960s
build 3: 40.256s
build 4: 40.355s

mvn compile -T1C -Daether.syncContext.named.factory=rwlock-redisson
build 1: 45.003s
build 2: 45.059s
build 3: 44.870s
build 4: 45.714s

mvn compile -T1C -Daether.syncContext.named.factory=semaphore-redisson
build 1: 42.151s
build 2: 43.388s
build 3: 43.598s
build 4: 44.008s

I did run into an issue running these tests, I've opened https://issues.apache.org/jira/browse/MRESOLVER-189 about it.

@jira-importer
Copy link

Resolve #870

1 similar comment
@jira-importer
Copy link

Resolve #870

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants