This is an automated email from the ASF dual-hosted git repository.

nehapawar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new e6dfe31  Move instantiation of objects which depend on 
_helixResourceManager to after invocation of PinotHelixResourceManager::start 
(#3713)
e6dfe31 is described below

commit e6dfe310c2d53fa4031d63ee5cb7015b29762d3e
Author: Neha Pawar <[email protected]>
AuthorDate: Fri Jan 18 13:22:32 2019 -0800

    Move instantiation of objects which depend on _helixResourceManager to 
after invocation of PinotHelixResourceManager::start (#3713)
    
    Certain objects which are instantiated in the ControllerStarter need an 
instance of PinotHelixResourceManager. Even though PinotHelixResourceManager is 
instantiated in the ControllerStarter constructor right at the beginning, it 
cannot be used until PinotHelixResourceManager::start is invoked. The start is 
invoked in ControllerStarter::start(). Every object that depends on 
PinotHelixResourceManager should be created after this invocation of start
    This PR moves all the dependents to after PinotHelixResourceManager being 
ready
---
 .../apache/pinot/controller/ControllerStarter.java | 71 +++++++++-------
 .../helix/ControllerPeriodicTaskStarterTest.java   | 97 ++++++++++++++++++++++
 .../pinot/controller/helix/ControllerTest.java     | 18 ++--
 3 files changed, 149 insertions(+), 37 deletions(-)

diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
index 370a13c..5d1546e 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
@@ -18,6 +18,7 @@
  */
 package org.apache.pinot.controller;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.primitives.Longs;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.yammer.metrics.core.MetricsRegistry;
@@ -79,35 +80,31 @@ public class ControllerStarter {
   private final ControllerConf _config;
   private final ControllerAdminApiApplication _adminApp;
   private final PinotHelixResourceManager _helixResourceManager;
-  private final RetentionManager _retentionManager;
   private final MetricsRegistry _metricsRegistry;
   private final ControllerMetrics _controllerMetrics;
-  private final PinotRealtimeSegmentManager _realtimeSegmentsManager;
-  private final SegmentStatusChecker _segmentStatusChecker;
   private final ExecutorService _executorService;
-  private final ControllerPeriodicTaskScheduler 
_controllerPeriodicTaskScheduler;
 
   // Can only be constructed after resource manager getting started
   private OfflineSegmentIntervalChecker _offlineSegmentIntervalChecker;
   private RealtimeSegmentValidationManager _realtimeSegmentValidationManager;
   private BrokerResourceValidationManager _brokerResourceValidationManager;
   private RealtimeSegmentRelocator _realtimeSegmentRelocator;
-  private PinotHelixTaskResourceManager _helixTaskResourceManager;
+  private RetentionManager _retentionManager;
+  private SegmentStatusChecker _segmentStatusChecker;
   private PinotTaskManager _taskManager;
+  private ControllerPeriodicTaskScheduler _controllerPeriodicTaskScheduler;
+  private PinotHelixTaskResourceManager _helixTaskResourceManager;
+  private PinotRealtimeSegmentManager _realtimeSegmentsManager;
 
   public ControllerStarter(ControllerConf conf) {
     _config = conf;
     _adminApp = new 
ControllerAdminApiApplication(_config.getQueryConsoleWebappPath(), 
_config.getQueryConsoleUseHttps());
+    // Do not use this before the invocation of {@link 
PinotHelixResourceManager::start()}, which happens in {@link 
ControllerStarter::start()}
     _helixResourceManager = new PinotHelixResourceManager(_config);
-    _retentionManager = new RetentionManager(_helixResourceManager, _config);
     _metricsRegistry = new MetricsRegistry();
     _controllerMetrics = new ControllerMetrics(_metricsRegistry);
-    _realtimeSegmentsManager = new 
PinotRealtimeSegmentManager(_helixResourceManager);
     _executorService =
         Executors.newCachedThreadPool(new 
ThreadFactoryBuilder().setNameFormat("restapi-multiget-thread-%d").build());
-    _segmentStatusChecker = new SegmentStatusChecker(_helixResourceManager, 
_config, _controllerMetrics);
-    _realtimeSegmentRelocator = new 
RealtimeSegmentRelocator(_helixResourceManager, _config);
-    _controllerPeriodicTaskScheduler = new ControllerPeriodicTaskScheduler();
   }
 
   public PinotHelixResourceManager getHelixResourceManager() {
@@ -134,6 +131,8 @@ public class ControllerStarter {
     return _taskManager;
   }
 
+
+
   public void start() {
     LOGGER.info("Starting Pinot controller");
 
@@ -184,31 +183,14 @@ public class ControllerStarter {
     // Helix resource manager must be started in order to create 
PinotLLCRealtimeSegmentManager
     LOGGER.info("Starting realtime segment manager");
     PinotLLCRealtimeSegmentManager.create(_helixResourceManager, _config, 
_controllerMetrics);
+    _realtimeSegmentsManager = new 
PinotRealtimeSegmentManager(_helixResourceManager);
     _realtimeSegmentsManager.start(_controllerMetrics);
 
     // Setting up periodic tasks
-    LOGGER.info("Setting up periodic tasks");
-    List<PeriodicTask> periodicTasks = new ArrayList<>();
-    _taskManager = new PinotTaskManager(_helixTaskResourceManager, 
_helixResourceManager, _config, _controllerMetrics);
-    periodicTasks.add(_taskManager);
-    periodicTasks.add(_retentionManager);
-    _offlineSegmentIntervalChecker =
-        new OfflineSegmentIntervalChecker(_config, _helixResourceManager, new 
ValidationMetrics(_metricsRegistry));
-    _realtimeSegmentValidationManager =
-        new RealtimeSegmentValidationManager(_config, _helixResourceManager, 
PinotLLCRealtimeSegmentManager.getInstance(),
-            new ValidationMetrics(_metricsRegistry));
-    _brokerResourceValidationManager =
-        new BrokerResourceValidationManager(_config, _helixResourceManager);
-
-    periodicTasks.add(_offlineSegmentIntervalChecker);
-    periodicTasks.add(_realtimeSegmentValidationManager);
-    periodicTasks.add(_brokerResourceValidationManager);
-    periodicTasks.add(_segmentStatusChecker);
-    periodicTasks.add(_realtimeSegmentRelocator);
-
+    List<PeriodicTask> controllerPeriodicTasks = 
setupControllerPeriodicTasks();
     LOGGER.info("Init controller periodic tasks scheduler");
-    _controllerPeriodicTaskScheduler.init(periodicTasks);
-
+    _controllerPeriodicTaskScheduler = new ControllerPeriodicTaskScheduler();
+    _controllerPeriodicTaskScheduler.init(controllerPeriodicTasks);
 
     LOGGER.info("Creating rebalance segments factory");
     RebalanceSegmentStrategyFactory.createInstance(helixManager);
@@ -312,6 +294,32 @@ public class ControllerStarter {
     _controllerMetrics.initializeGlobalMeters();
   }
 
+  @VisibleForTesting
+  protected List<PeriodicTask> setupControllerPeriodicTasks() {
+    LOGGER.info("Setting up periodic tasks");
+    List<PeriodicTask> periodicTasks = new ArrayList<>();
+    _taskManager = new PinotTaskManager(_helixTaskResourceManager, 
_helixResourceManager, _config, _controllerMetrics);
+    periodicTasks.add(_taskManager);
+    _retentionManager = new RetentionManager(_helixResourceManager, _config);
+    periodicTasks.add(_retentionManager);
+    _offlineSegmentIntervalChecker =
+        new OfflineSegmentIntervalChecker(_config, _helixResourceManager, new 
ValidationMetrics(_metricsRegistry));
+    periodicTasks.add(_offlineSegmentIntervalChecker);
+    _realtimeSegmentValidationManager =
+        new RealtimeSegmentValidationManager(_config, _helixResourceManager, 
PinotLLCRealtimeSegmentManager.getInstance(),
+            new ValidationMetrics(_metricsRegistry));
+    periodicTasks.add(_realtimeSegmentValidationManager);
+    _brokerResourceValidationManager =
+        new BrokerResourceValidationManager(_config, _helixResourceManager);
+    periodicTasks.add(_brokerResourceValidationManager);
+    _segmentStatusChecker = new SegmentStatusChecker(_helixResourceManager, 
_config, _controllerMetrics);
+    periodicTasks.add(_segmentStatusChecker);
+    _realtimeSegmentRelocator = new 
RealtimeSegmentRelocator(_helixResourceManager, _config);
+    periodicTasks.add(_realtimeSegmentRelocator);
+
+    return periodicTasks;
+  }
+
   public void stop() {
     try {
       LOGGER.info("Stopping controller leadership manager");
@@ -330,7 +338,6 @@ public class ControllerStarter {
       LOGGER.info("Stopping realtime segment manager");
       _realtimeSegmentsManager.stop();
 
-
       LOGGER.info("Stopping resource manager");
       _helixResourceManager.stop();
 
diff --git 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerPeriodicTaskStarterTest.java
 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerPeriodicTaskStarterTest.java
new file mode 100644
index 0000000..9620139
--- /dev/null
+++ 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerPeriodicTaskStarterTest.java
@@ -0,0 +1,97 @@
+/**
+ * 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.pinot.controller.helix;
+
+import java.util.List;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.controller.ControllerStarter;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.core.periodictask.PeriodicTask;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+public class ControllerPeriodicTaskStarterTest extends ControllerTest {
+  private MockControllerStarter _mockControllerStarter;
+
+  @BeforeClass
+  public void setup() {
+    startZk();
+  }
+
+  /**
+   * Test that controller starts up and helixResourceManager is non null 
before initiating periodic tasks
+   */
+  @Test
+  public void testHelixResourceManagerDuringControllerStart() {
+    startController();
+  }
+
+  @AfterClass
+  public void teardown() {
+    stopController();
+  }
+
+  @Override
+  protected void startControllerStarter(ControllerConf config) {
+    _mockControllerStarter = new MockControllerStarter(config);
+    _mockControllerStarter.start();
+    _helixResourceManager = _mockControllerStarter.getHelixResourceManager();
+  }
+
+  @Override
+  protected void stopControllerStarter() {
+    Assert.assertNotNull(_mockControllerStarter);
+
+    _mockControllerStarter.stop();
+    _mockControllerStarter = null;
+  }
+
+  private class MockControllerStarter extends ControllerStarter {
+
+    private static final int NUM_PERIODIC_TASKS = 7;
+
+    private List<PeriodicTask> _controllerPeriodicTasks;
+
+    public MockControllerStarter(ControllerConf conf) {
+      super(conf);
+    }
+
+    @Override
+    protected List<PeriodicTask> setupControllerPeriodicTasks() {
+      PinotHelixResourceManager helixResourceManager = 
getHelixResourceManager();
+      Assert.assertNotNull(helixResourceManager);
+      Assert.assertNotNull(helixResourceManager.getHelixAdmin());
+      Assert.assertNotNull(helixResourceManager.getHelixZkManager());
+      Assert.assertNotNull(helixResourceManager.getHelixClusterName());
+      Assert.assertNotNull(helixResourceManager.getPropertyStore());
+
+      _controllerPeriodicTasks = super.setupControllerPeriodicTasks();
+      Assert.assertNotNull(_controllerPeriodicTasks);
+      Assert.assertEquals(_controllerPeriodicTasks.size(), NUM_PERIODIC_TASKS);
+      return _controllerPeriodicTasks;
+    }
+
+    List<PeriodicTask> getControllerPeriodicTasks() {
+      return _controllerPeriodicTasks;
+    }
+   }
+}
diff --git 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
index 9ed2859..a4ca263 100644
--- 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
+++ 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
@@ -127,22 +127,30 @@ public abstract class ControllerTest {
       _zkClient.deleteRecursive("/" + helixClusterName);
     }
 
-    _controllerStarter = new ControllerStarter(config);
-    _controllerStarter.start();
+    startControllerStarter(config);
 
-    _helixResourceManager = _controllerStarter.getHelixResourceManager();
     _helixManager = _helixResourceManager.getHelixZkManager();
     _helixAdmin = _helixResourceManager.getHelixAdmin();
     _propertyStore = _helixResourceManager.getPropertyStore();
   }
 
+  protected void startControllerStarter(ControllerConf config) {
+    _controllerStarter = new ControllerStarter(config);
+    _controllerStarter.start();
+    _helixResourceManager = _controllerStarter.getHelixResourceManager();
+  }
+
   protected void stopController() {
+    stopControllerStarter();
+    FileUtils.deleteQuietly(new File(_controllerDataDir));
+    _zkClient.close();
+  }
+
+  protected void stopControllerStarter() {
     Assert.assertNotNull(_controllerStarter);
 
     _controllerStarter.stop();
     _controllerStarter = null;
-    FileUtils.deleteQuietly(new File(_controllerDataDir));
-    _zkClient.close();
   }
 
   protected Schema createDummySchema(String tableName) {


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to