MODE-1180 Starting the JcrEngine starts repositories in parallel #114

Merged
merged 5 commits into from Jun 1, 2011

2 participants

@rhauch
The ModeShape project member

When starting the JcrEngine and requesting that the repositories be initialized now performs the initialization in parallel (up to 4 at a time). There is also now an alternative method that accepts a timeout specification, after which the method will return even if the repositories are not all initialized. Note, however, that in such cases, getRepository(String) will block until all repositories are initialized.

A new unit test was added, and all unit and integration tests pass.

rhauch added some commits May 31, 2011
@rhauch rhauch MODE-1178 Corrected a few compiler warnings 1154649
@rhauch rhauch MODE-1180 Starting the JcrEngine starts repositories in parallel
When starting the JcrEngine and requesting that the repositories be initialized now performs the initialization in parallel (up to 4 at a time). There is also now an alternative method that accepts a timeout specification, after which the method will return even if the repositories are not all initialized. Note, however, that in such cases, getRepository(String) will block until all repositories are initialized.

A new unit test was added, and all unit and integration tests pass.
992c138
@bcarothers-xx

This is nitpicky, but you might want to add a note that a negative value for this parameter indicates that there is no timeout.

The ModeShape project member

Very good point.

rhauch added some commits Jun 1, 2011
@rhauch rhauch MODE-1180 Improved JavaDoc for recently-added method
The JavaDoc for the JcrEngine.start(boolean,long,TimeUnit) was improved to detail how to specify "infinite wait".
e180ea8
@rhauch rhauch MODE-1180 Improved the locking mechanism of JcrEngine
The locking within JcrEngine to guarantee concurrent access to the repositories was a bit primitive and simplistic - the initialization of all repositories locked out all access to other repositories.
This new approach allows the initialization to proceed without maintaining the locks. In fact, any operation that uses a JcrRepository will lock the engine only when obtaining the JcrRepositoryHolder. Once the holder is obtained, the getting of the JcrRepository will not require the lock, but may block until the repository is initialized.
d536a62
@rhauch rhauch commented on the diff Jun 1, 2011
...pe-jcr/src/main/java/org/modeshape/jcr/JcrEngine.java
@@ -84,7 +91,7 @@ public class JcrEngine extends ModeShapeEngine implements Repositories {
private static final Logger log = Logger.getLogger(ModeShapeEngine.class);
- private final Map<String, JcrRepository> repositories;
+ private final Map<String, JcrRepositoryHolder> repositories;
@rhauch
The ModeShape project member
rhauch added a line comment Jun 1, 2011

@bcarothers The JcrEngine is now managing a map of JcrRepositoryHolder instances that basically wrap the JcrRepository (or a Future that may be initializing the repository). The single lock is still used to ensure a single thread operates on the engine's map at any one time, but that lock is there merely to obtain the correct JcrRepositoryHolder instance(s), and the lock is then released. Obtaining the actual JcrRepository (which may block if the repository is still be initialized) is no longer done within the scope of the lock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rhauch rhauch commented on an outdated diff Jun 1, 2011
...pe-jcr/src/main/java/org/modeshape/jcr/JcrEngine.java
try {
- getRepository(repositoryName);
- } catch (Throwable t) {
- // Record this in the problems ...
- this.problems.addError(t, JcrI18n.errorStartingRepositoryCheckConfiguration, repositoryName, t.getMessage());
+ repositoriesLock.lock();
@rhauch
The ModeShape project member
rhauch added a line comment Jun 1, 2011

@bcarothers The repositories lock is held while a holder for each repository is created and put into the map and the callable is submitted into the queue. But because the actual initialization is being done by threads in the ExecutorService, the initialization completes outside of the lock scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rhauch rhauch commented on an outdated diff Jun 1, 2011
...pe-jcr/src/main/java/org/modeshape/jcr/JcrEngine.java
+ // Now wait for the all the startups to complete ...
+ try {
+ if (timeout < 0L) {
+ latch.await();
+ } else {
+ latch.await(timeout, timeoutUnit);
+ }
+ } catch (InterruptedException e) {
+ this.problems.addError(e, JcrI18n.startingAllRepositoriesWasInterrupted, e.getMessage());
+ }
+
+ } else {
+ // Otherwise there's just 0 or 1, so simple to start in serial ...
+ for (String repositoryName : repositoryNames) {
+ try {
+ getRepository(repositoryName);
@rhauch
The ModeShape project member
rhauch added a line comment Jun 1, 2011

@bcarothers This is all done within the same thread, and this does block; see the comment in the getRepository(String) method for reasons why.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rhauch
The ModeShape project member

@bcarothers I think this is a pretty good implementation of what we talked about on IRC.

@rhauch rhauch MODE-1180 Improved the locking mechanism of JcrEngine
Made additional improvements to simplify the logic and to handle cases where one thread calls getRepository(String) but, before that repository is initialized, another thread calls the same method with the same repository name. Before these changes, this would cause the second thread to return a null reference.
ab16049
@bcarothers-xx
The ModeShape project member

I like the approach with the Futures. It looks good to me.

@rhauch rhauch merged commit ab16049 into ModeShape:master Jun 1, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment