You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by fr...@apache.org on 2016/12/07 09:52:03 UTC

svn commit: r1773038 - in /jackrabbit/oak/trunk: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/ oak-pojosr/ oak-pojosr/src/main/java/org/apache/jackrabbit/oak/run/osgi/ oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/ oak-...

Author: frm
Date: Wed Dec  7 09:52:03 2016
New Revision: 1773038

URL: http://svn.apache.org/viewvc?rev=1773038&view=rev
Log:
OAK-5223 - Let SegmentNodeStoreService unregister services correctly

SegmentNodeStoreService now uniformly unregisters the services it owns, even
when an external BlobStore is used. Moreover, the reference to the BlobStore
has been changed from dynamic and reluctant to static and greedy to simplify
the implementation of the SegmentNodeStoreService.

Added:
    jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/TarSegmentNodeStoreConfigTest.groovy
    jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentStoreProvider.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractBlobTrackerRegistrationTest.java
    jackrabbit/oak/trunk/oak-pojosr/pom.xml
    jackrabbit/oak/trunk/oak-pojosr/src/main/java/org/apache/jackrabbit/oak/run/osgi/OakOSGiRepositoryFactory.java
    jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreService.java
    jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreServiceTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractBlobTrackerRegistrationTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractBlobTrackerRegistrationTest.java?rev=1773038&r1=1773037&r2=1773038&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractBlobTrackerRegistrationTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractBlobTrackerRegistrationTest.java Wed Dec  7 09:52:03 2016
@@ -19,6 +19,12 @@
 
 package org.apache.jackrabbit.oak.plugins.blob;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
 import java.io.File;
 
 import javax.annotation.Nullable;
@@ -40,12 +46,6 @@ import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.osgi.framework.ServiceRegistration;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-
 /**
  * Tests OSGi registration for {@link BlobTrackingStore}.
  */
@@ -72,9 +72,8 @@ public abstract class AbstractBlobTracke
 
     @Test
     public void registerBlobTrackingStore() throws Exception {
-        registerNodeStoreService();
-        assertServiceNotActivated();
         registerTrackingBlobStore();
+        registerNodeStoreService();
         assertServiceActivated();
 
         BlobStore blobStore = context.getService(BlobStore.class);
@@ -86,9 +85,8 @@ public abstract class AbstractBlobTracke
 
     @Test
     public void reRegisterBlobTrackingStore() throws Exception {
-        registerNodeStoreService();
-        assertServiceNotActivated();
         registerTrackingBlobStore();
+        registerNodeStoreService();
         assertServiceActivated();
 
         BlobStore blobStore = context.getService(BlobStore.class);

Modified: jackrabbit/oak/trunk/oak-pojosr/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-pojosr/pom.xml?rev=1773038&r1=1773037&r2=1773038&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-pojosr/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-pojosr/pom.xml Wed Dec  7 09:52:03 2016
@@ -94,6 +94,11 @@
     </dependency>
     <dependency>
       <groupId>org.apache.jackrabbit</groupId>
+      <artifactId>oak-segment-tar</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.jackrabbit</groupId>
       <artifactId>oak-jcr</artifactId>
       <version>${project.version}</version>
     </dependency>

Modified: jackrabbit/oak/trunk/oak-pojosr/src/main/java/org/apache/jackrabbit/oak/run/osgi/OakOSGiRepositoryFactory.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-pojosr/src/main/java/org/apache/jackrabbit/oak/run/osgi/OakOSGiRepositoryFactory.java?rev=1773038&r1=1773037&r2=1773038&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-pojosr/src/main/java/org/apache/jackrabbit/oak/run/osgi/OakOSGiRepositoryFactory.java (original)
+++ jackrabbit/oak/trunk/oak-pojosr/src/main/java/org/apache/jackrabbit/oak/run/osgi/OakOSGiRepositoryFactory.java Wed Dec  7 09:52:03 2016
@@ -19,6 +19,8 @@
 
 package org.apache.jackrabbit.oak.run.osgi;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.lang.management.ManagementFactory;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
@@ -64,8 +66,6 @@ import org.osgi.util.tracker.ServiceTrac
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 /**
  * RepositoryFactory which constructs an instance of Oak repository. Thi factory supports following
  * parameters
@@ -133,12 +133,15 @@ public class OakOSGiRepositoryFactory im
     public static final String REPOSITORY_ENV_SPRING_BOOT =
             "org.apache.jackrabbit.oak.repository.springBootMode";
 
-    public static final String REPOSITORY_BUNDLE_FILTER_DEFAULT = "(|" +
+    public static final String REPOSITORY_BUNDLE_FILTER_DEFAULT = "(&" +
+            "(|" +
             "(Bundle-SymbolicName=org.apache.jackrabbit*)" +
             "(Bundle-SymbolicName=org.apache.sling*)" +
             "(Bundle-SymbolicName=org.apache.felix*)" +
             "(Bundle-SymbolicName=org.apache.aries*)" +
             "(Bundle-SymbolicName=groovy-all)" +
+            ")" +
+            "(!(Bundle-SymbolicName=org.apache.jackrabbit.oak-segment-tar))" +
             ")";
 
     /**

Added: jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/TarSegmentNodeStoreConfigTest.groovy
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/TarSegmentNodeStoreConfigTest.groovy?rev=1773038&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/TarSegmentNodeStoreConfigTest.groovy (added)
+++ jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/TarSegmentNodeStoreConfigTest.groovy Wed Dec  7 09:52:03 2016
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jackrabbit.oak.run.osgi
+
+import org.apache.felix.connect.launch.PojoServiceRegistry
+import org.apache.jackrabbit.oak.spi.blob.BlobStore
+import org.apache.jackrabbit.oak.spi.state.NodeStore
+import org.junit.Before
+import org.junit.Test
+
+import static org.junit.Assert.assertNotNull
+import static org.junit.Assert.assertNull
+import static org.mockito.Mockito.mock
+
+class TarSegmentNodeStoreConfigTest extends AbstractRepositoryFactoryTest {
+
+    private static final String SEGMENT_TAR_BUNDLE_FILTER = "(&" +
+            "(|" +
+            "(Bundle-SymbolicName=org.apache.jackrabbit*)" +
+            "(Bundle-SymbolicName=org.apache.sling*)" +
+            "(Bundle-SymbolicName=org.apache.felix*)" +
+            "(Bundle-SymbolicName=org.apache.aries*)" +
+            "(Bundle-SymbolicName=groovy-all)" +
+            ")" +
+            "(!(Bundle-SymbolicName=org.apache.jackrabbit.oak-segment))" +
+            ")"
+
+    private PojoServiceRegistry registry
+
+    @Before
+    void adjustConfig() {
+        config[OakOSGiRepositoryFactory.REPOSITORY_BUNDLE_FILTER] = SEGMENT_TAR_BUNDLE_FILTER
+        registry = repositoryFactory.initializeServiceRegistry(config)
+    }
+
+    @Override
+    protected PojoServiceRegistry getRegistry() {
+        return registry
+    }
+
+    @Test
+    void testDynamicBlobStore() {
+        createConfig([
+                'org.apache.jackrabbit.oak.segment.SegmentNodeStoreService': [
+                        "customBlobStore": true
+                ]
+        ])
+        assertNull(registry.getServiceReference(NodeStore.class.name))
+        def registration = registry.registerService(BlobStore.class.name, mock(BlobStore.class), null)
+        assertNotNull(getServiceWithWait(NodeStore.class))
+        registration.unregister()
+        assertNull(registry.getServiceReference(NodeStore.class.name))
+    }
+
+}

Added: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentStoreProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentStoreProvider.java?rev=1773038&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentStoreProvider.java (added)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentStoreProvider.java Wed Dec  7 09:52:03 2016
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jackrabbit.oak.segment;
+
+/**
+ * A {@link SegmentStoreProvider} that returns a {@link SegmentStore} instance
+ * provided by the user.
+ */
+class DefaultSegmentStoreProvider implements SegmentStoreProvider {
+
+    private final SegmentStore segmentStore;
+
+    DefaultSegmentStoreProvider(SegmentStore segmentStore) {
+        this.segmentStore = segmentStore;
+    }
+
+    @Override
+    public SegmentStore getSegmentStore() {
+        return segmentStore;
+    }
+
+}

Propchange: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentStoreProvider.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreService.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreService.java?rev=1773038&r1=1773037&r2=1773038&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreService.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreService.java Wed Dec  7 09:52:03 2016
@@ -18,13 +18,13 @@
  */
 package org.apache.jackrabbit.oak.segment;
 
-import static com.google.common.base.Preconditions.checkState;
 import static java.util.Collections.emptyMap;
 import static org.apache.jackrabbit.oak.commons.PropertiesUtil.toBoolean;
 import static org.apache.jackrabbit.oak.commons.PropertiesUtil.toInteger;
 import static org.apache.jackrabbit.oak.commons.PropertiesUtil.toLong;
 import static org.apache.jackrabbit.oak.osgi.OsgiUtil.lookupConfigurationThenFramework;
 import static org.apache.jackrabbit.oak.segment.SegmentNotFoundExceptionListener.IGNORE_SNFE;
+import static org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.DISABLE_ESTIMATION_DEFAULT;
 import static org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.FORCE_TIMEOUT_DEFAULT;
 import static org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.GC_PROGRESS_LOG_DEFAULT;
 import static org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.MEMORY_THRESHOLD_DEFAULT;
@@ -32,13 +32,11 @@ import static org.apache.jackrabbit.oak.
 import static org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.RETAINED_GENERATIONS_DEFAULT;
 import static org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.RETRY_COUNT_DEFAULT;
 import static org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.SIZE_DELTA_ESTIMATION_DEFAULT;
-import static org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.DISABLE_ESTIMATION_DEFAULT;
 import static org.apache.jackrabbit.oak.segment.file.FileStoreBuilder.fileStoreBuilder;
 import static org.apache.jackrabbit.oak.spi.blob.osgi.SplitBlobStoreService.ONLY_STANDALONE_TARGET;
 import static org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils.registerMBean;
 
 import java.io.ByteArrayInputStream;
-import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -48,6 +46,8 @@ import java.util.Hashtable;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
+import com.google.common.base.Strings;
+import com.google.common.base.Supplier;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.ConfigurationPolicy;
@@ -56,6 +56,7 @@ import org.apache.felix.scr.annotations.
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.ReferencePolicy;
+import org.apache.felix.scr.annotations.ReferencePolicyOption;
 import org.apache.jackrabbit.commons.SimpleValueFactory;
 import org.apache.jackrabbit.oak.api.Descriptors;
 import org.apache.jackrabbit.oak.api.jmx.CacheStatsMBean;
@@ -85,12 +86,9 @@ import org.apache.jackrabbit.oak.segment
 import org.apache.jackrabbit.oak.segment.file.InvalidFileStoreVersionException;
 import org.apache.jackrabbit.oak.spi.blob.BlobStore;
 import org.apache.jackrabbit.oak.spi.blob.GarbageCollectableBlobStore;
-import org.apache.jackrabbit.oak.spi.commit.Observable;
-import org.apache.jackrabbit.oak.spi.commit.Observer;
 import org.apache.jackrabbit.oak.spi.gc.GCMonitor;
 import org.apache.jackrabbit.oak.spi.gc.GCMonitorTracker;
 import org.apache.jackrabbit.oak.spi.state.NodeStore;
-import org.apache.jackrabbit.oak.spi.state.ProxyNodeStore;
 import org.apache.jackrabbit.oak.spi.state.RevisionGC;
 import org.apache.jackrabbit.oak.spi.state.RevisionGCMBean;
 import org.apache.jackrabbit.oak.spi.whiteboard.CompositeRegistration;
@@ -105,9 +103,6 @@ import org.osgi.service.component.Compon
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Strings;
-import com.google.common.base.Supplier;
-
 /**
  * An OSGi wrapper for the segment node store.
  */
@@ -120,10 +115,7 @@ import com.google.common.base.Supplier;
                 "should be done via file system based config file and this view should ONLY be used to determine which " +
                 "options are supported"
 )
-public class SegmentNodeStoreService extends ProxyNodeStore
-        implements Observable, SegmentStoreProvider {
-
-    public static final String NAME = "name";
+public class SegmentNodeStoreService {
 
     @Property(
             label = "Directory",
@@ -273,20 +265,20 @@ public class SegmentNodeStoreService ext
 
     private final Logger log = LoggerFactory.getLogger(getClass());
 
-    private String name;
-
     private FileStore store;
 
-    private volatile SegmentNodeStore segmentNodeStore;
-
     private ObserverTracker observerTracker;
 
     private GCMonitorTracker gcMonitor;
 
     private ComponentContext context;
 
-    @Reference(cardinality = ReferenceCardinality.OPTIONAL_UNARY,
-            policy = ReferencePolicy.DYNAMIC, target = ONLY_STANDALONE_TARGET)
+    @Reference(
+            cardinality = ReferenceCardinality.OPTIONAL_UNARY,
+            policy = ReferencePolicy.STATIC,
+            policyOption = ReferencePolicyOption.GREEDY,
+            target = ONLY_STANDALONE_TARGET
+    )
     private volatile BlobStore blobStore;
 
     @Reference
@@ -325,12 +317,6 @@ public class SegmentNodeStoreService ext
     )
     public static final String PROP_BLOB_SNAPSHOT_INTERVAL = "blobTrackSnapshotIntervalInSecs";
 
-    @Override
-    protected SegmentNodeStore getNodeStore() {
-        checkState(segmentNodeStore != null, "service must be activated when used");
-        return segmentNodeStore;
-    }
-
     @Activate
     public void activate(ComponentContext context) throws IOException {
         this.context = context;
@@ -338,59 +324,8 @@ public class SegmentNodeStoreService ext
 
         if (blobStore == null && customBlobStore) {
             log.info("BlobStore use enabled. SegmentNodeStore would be initialized when BlobStore would be available");
-        } else {
-            registerNodeStore();
-        }
-    }
-
-    protected void bindBlobStore(BlobStore blobStore) throws IOException {
-        this.blobStore = blobStore;
-        registerNodeStore();
-    }
-
-    protected void unbindBlobStore(BlobStore blobStore){
-        this.blobStore = null;
-        unregisterNodeStore();
-    }
-
-    @Deactivate
-    public void deactivate() {
-        unregisterNodeStore();
-
-        synchronized (this) {
-            if (observerTracker != null) {
-                observerTracker.stop();
-            }
-            if (gcMonitor != null) {
-                gcMonitor.stop();
-            }
-            segmentNodeStore = null;
-
-            if (store != null) {
-                store.close();
-                store = null;
-            }
-        }
-    }
-
-    private synchronized void registerNodeStore() throws IOException {
-        if (!registerSegmentStore()) {
             return;
         }
-        if (toBoolean(property(STANDBY), false)) {
-            return;
-        }
-        Dictionary<String, Object> props = new Hashtable<String, Object>();
-        props.put(Constants.SERVICE_PID, SegmentNodeStore.class.getName());
-        props.put("oak.nodestore.description", new String[] {"nodeStoreType=segment"});
-        storeRegistration = context.getBundleContext().registerService(NodeStore.class.getName(), this, props);
-    }
-
-    private boolean registerSegmentStore() throws IOException {
-        if (context == null) {
-            log.info("Component still not activated. Ignoring the initialization call");
-            return false;
-        }
 
         OsgiWhiteboard whiteboard = new OsgiWhiteboard(context.getBundleContext());
 
@@ -420,7 +355,7 @@ public class SegmentNodeStoreService ext
             log.info("Initializing SegmentNodeStore with BlobStore [{}]", blobStore);
             builder.withBlobStore(blobStore);
         }
-        
+
         if (toBoolean(property(STANDBY), true)) {
             builder.withSnfeListener(IGNORE_SNFE);
         }
@@ -429,7 +364,7 @@ public class SegmentNodeStoreService ext
             store = builder.build();
         } catch (InvalidFileStoreVersionException e) {
             log.error("The segment store data is not compatible with the current version. Please use oak-segment or a different version of oak-segment-tar.");
-            return false;
+            return;
         }
 
         // Expose stats about the segment cache
@@ -449,7 +384,7 @@ public class SegmentNodeStoreService ext
         registrations.add(registerMBean(
                 whiteboard,
                 CacheStatsMBean.class,
-                stringCacheStats,CacheStats.TYPE,
+                stringCacheStats, CacheStats.TYPE,
                 stringCacheStats.getName()
         ));
 
@@ -457,35 +392,35 @@ public class SegmentNodeStoreService ext
         registrations.add(registerMBean(
                 whiteboard,
                 CacheStatsMBean.class,
-                templateCacheStats,CacheStats.TYPE,
+                templateCacheStats, CacheStats.TYPE,
                 templateCacheStats.getName()
         ));
 
         CacheStatsMBean stringDeduplicationCacheStats = store.getStringDeduplicationCacheStats();
         if (stringDeduplicationCacheStats != null) {
             registrations.add(registerMBean(
-            whiteboard,
-            CacheStatsMBean.class,
-            stringDeduplicationCacheStats,CacheStats.TYPE,
-            stringDeduplicationCacheStats.getName()));
+                    whiteboard,
+                    CacheStatsMBean.class,
+                    stringDeduplicationCacheStats, CacheStats.TYPE,
+                    stringDeduplicationCacheStats.getName()));
         }
 
         CacheStatsMBean templateDeduplicationCacheStats = store.getTemplateDeduplicationCacheStats();
         if (templateDeduplicationCacheStats != null) {
             registrations.add(registerMBean(
-            whiteboard,
-            CacheStatsMBean.class,
-            templateDeduplicationCacheStats,CacheStats.TYPE,
-            templateDeduplicationCacheStats.getName()));
+                    whiteboard,
+                    CacheStatsMBean.class,
+                    templateDeduplicationCacheStats, CacheStats.TYPE,
+                    templateDeduplicationCacheStats.getName()));
         }
 
         CacheStatsMBean nodeDeduplicationCacheStats = store.getNodeDeduplicationCacheStats();
         if (nodeDeduplicationCacheStats != null) {
             registrations.add(registerMBean(
-            whiteboard,
-            CacheStatsMBean.class,
-            nodeDeduplicationCacheStats,CacheStats.TYPE,
-            nodeDeduplicationCacheStats.getName()));
+                    whiteboard,
+                    CacheStatsMBean.class,
+                    nodeDeduplicationCacheStats, CacheStats.TYPE,
+                    nodeDeduplicationCacheStats.getName()));
         }
 
         // Listen for Executor services on the whiteboard
@@ -497,22 +432,24 @@ public class SegmentNodeStoreService ext
 
         final FileStoreGCMonitor fsgcm = new FileStoreGCMonitor(Clock.SIMPLE);
         registrations.add(new CompositeRegistration(
-            whiteboard.register(GCMonitor.class, fsgcm, emptyMap()),
-            registerMBean(
-                whiteboard,
-                SegmentRevisionGC.class,
-                new SegmentRevisionGCMBean(store, gcOptions, fsgcm),
-                SegmentRevisionGC.TYPE,
-                "Segment node store revision garbage collection"
-            )));
+                whiteboard.register(GCMonitor.class, fsgcm, emptyMap()),
+                registerMBean(
+                        whiteboard,
+                        SegmentRevisionGC.class,
+                        new SegmentRevisionGCMBean(store, gcOptions, fsgcm),
+                        SegmentRevisionGC.TYPE,
+                        "Segment node store revision garbage collection"
+                )));
 
         Runnable cancelGC = new Runnable() {
+
             @Override
             public void run() {
                 store.cancelGC();
             }
         };
         Supplier<String> statusMessage = new Supplier<String>() {
+
             @Override
             public String get() {
                 return fsgcm.getStatus();
@@ -538,18 +475,15 @@ public class SegmentNodeStoreService ext
 
         // register segment node store
 
-        Dictionary<?, ?> properties = context.getProperties();
-        name = String.valueOf(properties.get(NAME));
-
         final long blobGcMaxAgeInSecs = toLong(property(PROP_BLOB_GC_MAX_AGE), DEFAULT_BLOB_GC_MAX_AGE);
 
         SegmentNodeStore.SegmentNodeStoreBuilder segmentNodeStoreBuilder =
                 SegmentNodeStoreBuilders.builder(store)
-                .withStatisticsProvider(statisticsProvider);
+                        .withStatisticsProvider(statisticsProvider);
         if (toBoolean(property(STANDBY), false)) {
             segmentNodeStoreBuilder.dispatchChanges(false);
         }
-        segmentNodeStore = segmentNodeStoreBuilder.build();
+        SegmentNodeStore segmentNodeStore = segmentNodeStoreBuilder.build();
 
         observerTracker = new ObserverTracker(segmentNodeStore);
         observerTracker.start(context.getBundleContext());
@@ -622,18 +556,18 @@ public class SegmentNodeStoreService ext
                     "Segment node store blob garbage collection"
             ));
         }
-        
+
         // Expose an MBean for backup/restore operations
-        
+
         registrations.add(registerMBean(
                 whiteboard,
                 FileStoreBackupRestoreMBean.class,
-                new FileStoreBackupRestoreImpl(segmentNodeStore, store.getRevisions(), store.getReader(), getBackupDirectory(), executor), 
+                new FileStoreBackupRestoreImpl(segmentNodeStore, store.getRevisions(), store.getReader(), getBackupDirectory(), executor),
                 FileStoreBackupRestoreMBean.TYPE, "Segment node store backup/restore"
         ));
 
         // Expose statistics about the SegmentNodeStore
-        
+
         registrations.add(registerMBean(
                 whiteboard,
                 SegmentNodeStoreStatsMBean.class,
@@ -641,14 +575,55 @@ public class SegmentNodeStoreService ext
                 SegmentNodeStoreStatsMBean.TYPE,
                 "SegmentNodeStore statistics"
         ));
-        
+
         log.info("SegmentNodeStore initialized");
 
         // Register a factory service to expose the FileStore
 
-        providerRegistration = context.getBundleContext().registerService(SegmentStoreProvider.class.getName(), this, null);
+        providerRegistration = context.getBundleContext().registerService(
+                SegmentStoreProvider.class.getName(),
+                new DefaultSegmentStoreProvider(store),
+                null
+        );
+
+        if (toBoolean(property(STANDBY), false)) {
+            return;
+        }
 
-        return true;
+        Dictionary<String, Object> props = new Hashtable<String, Object>();
+        props.put(Constants.SERVICE_PID, SegmentNodeStore.class.getName());
+        props.put("oak.nodestore.description", new String[] {"nodeStoreType=segment"});
+        storeRegistration = context.getBundleContext().registerService(NodeStore.class.getName(), segmentNodeStore, props);
+    }
+
+    @Deactivate
+    public void deactivate() {
+        new CompositeRegistration(registrations).unregister();
+        registrations.clear();
+        if (providerRegistration != null) {
+            providerRegistration.unregister();
+            providerRegistration = null;
+        }
+        if (storeRegistration != null) {
+            storeRegistration.unregister();
+            storeRegistration = null;
+        }
+        if (executor != null) {
+            executor.stop();
+            executor = null;
+        }
+        if (observerTracker != null) {
+            observerTracker.stop();
+            observerTracker = null;
+        }
+        if (gcMonitor != null) {
+            gcMonitor.stop();
+            gcMonitor = null;
+        }
+        if (store != null) {
+            store.close();
+            store = null;
+        }
     }
 
     private SegmentGCOptions newGCOptions() {
@@ -675,22 +650,6 @@ public class SegmentNodeStoreService ext
                 .withGCNodeWriteMonitor(gcProgressLog);
     }
 
-    private void unregisterNodeStore() {
-        new CompositeRegistration(registrations).unregister();
-        if (providerRegistration != null) {
-            providerRegistration.unregister();
-            providerRegistration = null;
-        }
-        if (storeRegistration != null) {
-            storeRegistration.unregister();
-            storeRegistration = null;
-        }
-        if (executor != null) {
-            executor.stop();
-            executor = null;
-        }
-    }
-
     private File getBaseDirectory() {
         String directory = property(DIRECTORY);
 
@@ -779,26 +738,4 @@ public class SegmentNodeStoreService ext
         return lookupConfigurationThenFramework(context, name);
     }
 
-    /**
-     * needed for situations where you have to unwrap the
-     * SegmentNodeStoreService, to get the SegmentStore, like the failover
-     */
-    @Override
-    public SegmentStore getSegmentStore() {
-        return store;
-    }
-
-    //------------------------------------------------------------< Observable >---
-
-    @Override
-    public Closeable addObserver(Observer observer) {
-        return getNodeStore().addObserver(observer);
-    }
-
-    //------------------------------------------------------------< Object >--
-
-    @Override
-    public String toString() {
-        return name + ": " + segmentNodeStore;
-    }
 }

Modified: jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreServiceTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreServiceTest.java?rev=1773038&r1=1773037&r2=1773038&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreServiceTest.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreServiceTest.java Wed Dec  7 09:52:03 2016
@@ -105,41 +105,6 @@ public class SegmentNodeStoreServiceTest
         unregisterBlobStore();
     }
 
-    /**
-     * A NodeStore service should be registered when the "customBlobStore"
-     * configuration property is true and a BlobStore service becomes
-     * dynamically available.
-     */
-    @Test
-    public void testUseCustomBlobStoreWithDynamicBlobStoreActivation() {
-        registerSegmentNodeStoreService(true);
-        assertServiceNotActivated();
-
-        registerBlobStore();
-        assertServiceActivated();
-
-        unregisterSegmentNodeStoreService();
-        unregisterBlobStore();
-    }
-
-    /**
-     * A NodeStore service should be unregistered when the "customBlobStore"
-     * configuration property is true and a BlobStore service becomes
-     * dynamically unavailable.
-     */
-    @Test
-    public void testUseCustomBlobStoreWithDynamicBlobStoreDeactivation() {
-        registerBlobStore();
-
-        registerSegmentNodeStoreService(true);
-        assertServiceActivated();
-
-        unregisterBlobStore();
-        assertServiceNotActivated();
-
-        unregisterSegmentNodeStoreService();
-    }
-
     private SegmentNodeStoreService segmentNodeStoreService;
 
     protected void registerSegmentNodeStoreService(boolean customBlobStore) {