-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support assembling and archiving repositories in parallel #2850
Conversation
ad62fbd
to
c58f295
Compare
import org.apache.maven.plugins.annotations.Parameter; | ||
import org.eclipse.tycho.core.maven.AbstractP2Mojo; | ||
|
||
public abstract class AbstractRepositoryMojo extends AbstractP2Mojo { | ||
private static final Map<File, Lock> REPOSITORY_LOCK = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tycho has a FileLockService
can we probably reuse that here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into that, but since the FileLockService creates a lock-file in the directory to lock this is not usable in cases where for example a repository is zipped, because then the lock file ends up in the zipped repository.
Furthermore a inter-process file locking does not seem to be necessary because the resources are within the projects being build and the goal is only to prevent illegal concurrent access in the same build-process. One should not run multiple build processes at the same time for the same or even sub-directories any way. So having a file-locking seems to be too much for me and an in-memory locking is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But with #2853 we could simply add another method like FileLockService.lockForThisProcess
or similar that only does an ordinary in-process/memory locking without creating any files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into that, but since the FileLockService creates a lock-file in the directory to lock this is not usable in cases where for example a repository is zipped, because then the lock file ends up in the zipped repository.
Have you tested this? The zipping of repository is taking place in a separate step where the lock file should already be deleted (or can be deleted).
One should not run multiple build processes at the same time for the same or even sub-directories any way
I think you are aware of murphy's law ;-)
So having a file-locking seems to be too much for me and an in-memory locking is sufficient.
If we can reuse something that offers even more we should use that instead of inventing another approach that offers weaker guarantees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now you have refactored the lock do you think it could be used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into that, but since the FileLockService creates a lock-file in the directory to lock this is not usable in cases where for example a repository is zipped, because then the lock file ends up in the zipped repository.
Have you tested this? The zipping of repository is taking place in a separate step where the lock file should already be deleted (or can be deleted).
I haven't tried it yet. But with this zipping the repo would also be guarded with a lock-file, which would create the file before the zipping starts and deletes it only afterwards. And in between the directory content is zipped. Therefore expect that the lock-file will be included. But I'll try it.
One should not run multiple build processes at the same time for the same or even sub-directories any way
I think you are aware of murphy's law ;-)
It could definitively happen, but one should not do it. So its then the users fault. :)
So having a file-locking seems to be too much for me and an in-memory locking is sufficient.
If we can reuse something that offers even more we should use that instead of inventing another approach that offers weaker guarantees.
In generally that's right. But as written above, I'm not yet sure if this is really suitable. If it is not suitable, I would just add the method as something like lockVirtually()
to the FileLockerService
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do not loose anything using the function as-is ... any complication like "virtual" make it just confusing so if we zip the content it should be easy to exclude the lockfile anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would see the opposite as more complicated since the exclusion then would need to be kept in sync if the lock-file changes one day.
And which file is locked eventually is an implementation detail of the locker and consumer not interested in inter-process locking should not have to care about that.
55ed2de
to
80a94c1
Compare
Assembling and arching repositories is a relatively long running task and repository-projects are usually build as one of the last projects in the reactor. So the chances are relatively high, that (if multiple projects are present) multiple repositories are assembled at the same. Therefore it should be possible to build multiple repositories in parallel and there should not be one global lock for each repository-related mojo.
80a94c1
to
f934b1e
Compare
Assembling and arching repositories is a relatively long running task and repository-projects are usually build as one of the last projects in the reactor. So the chances are relatively high, that (if multiple projects are present) multiple repositories are assembled at the same. Therefore it should be possible to build multiple repositories in parallel and there should not be one global lock for each repository-related mojo.