HBASE-12559 Revert for addressing review comments

hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java

@@ -1253,12 +1253,6 @@ public interface Admin extends Abortable, Closeable {
    */
   CoprocessorRpcChannel coprocessorService(ServerName sn);
 
-  /**
-   * Update the configuration and trigger an online config change
-   * on the master
-   * @throws IOException
-   */
-  void updateMasterConfiguration() throws IOException;
 
   /**
    * Update the configuration and trigger an online config change

hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java

@@ -162,8 +162,6 @@ import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.TruncateTableRequ
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.TruncateTableResponse;
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UnassignRegionRequest;
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UnassignRegionResponse;
-import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UpdateMasterConfigurationRequest;
-import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UpdateMasterConfigurationResponse;
 import org.apache.hadoop.hbase.regionserver.RegionServerStoppedException;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.security.UserProvider;
@@ -1716,12 +1714,6 @@ class ConnectionManager {
           return stub.addColumn(controller, request);
         }
 
-        @Override
-        public UpdateMasterConfigurationResponse updateMasterConfiguration(RpcController controller,
-            UpdateMasterConfigurationRequest request) throws ServiceException {
-          return stub.updateMasterConfiguration(controller, request);
-        }
-
         @Override
         public DeleteColumnResponse deleteColumn(RpcController controller,
             DeleteColumnRequest request)

hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java

@@ -128,7 +128,6 @@ import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.SnapshotResponse;
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.StopMasterRequest;
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.TruncateTableRequest;
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UnassignRegionRequest;
-import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UpdateMasterConfigurationRequest;
 import org.apache.hadoop.hbase.quotas.QuotaFilter;
 import org.apache.hadoop.hbase.quotas.QuotaRetriever;
 import org.apache.hadoop.hbase.quotas.QuotaSettings;
@@ -3720,16 +3719,6 @@ public class HBaseAdmin implements Admin {
     return new RegionServerCoprocessorRpcChannel(connection, sn);
   }
 
-  @Override
-  public void updateMasterConfiguration() throws IOException {
-    try {
-      this.connection.getMaster().updateMasterConfiguration(null,
-        UpdateMasterConfigurationRequest.getDefaultInstance());
-    } catch (ServiceException e) {
-      throw ProtobufUtil.getRemoteException(e);
-    }
-  }
-
   @Override
   public void updateConfiguration(ServerName server) throws IOException {
     try {

hbase-protocol/src/main/protobuf/Master.proto

@@ -382,12 +382,6 @@ message SetQuotaRequest {
 message SetQuotaResponse {
 }
 
-message UpdateMasterConfigurationRequest {
-}
-
-message UpdateMasterConfigurationResponse {
-}
-
 service MasterService {
   /** Used by the client to get the number of regions that have received the updated schema */
   rpc GetSchemaAlterStatus(GetSchemaAlterStatusRequest)
@@ -602,7 +596,4 @@ service MasterService {
 
   /** Apply the new quota settings */
   rpc SetQuota(SetQuotaRequest) returns(SetQuotaResponse);
-
-  rpc UpdateMasterConfiguration(UpdateMasterConfigurationRequest)
-    returns(UpdateMasterConfigurationResponse);
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

@@ -1955,16 +1955,4 @@ public class HMaster extends HRegionServer implements MasterServices, Server {
     }
     return tableNames;
   }
-
-  /**
-   * Reload the configuration from disk.
-   */
-  public void updateConfiguration() {
-    LOG.info("Reloading the configuration from disk.");
-    // Reload the configuration from disk.
-    conf.reloadConfiguration();
-    synchronized (this.balancer) {
-      balancer.setConf(conf);
-    }
-  }
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java

@@ -144,7 +144,6 @@ import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.TruncateTableRequ
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.TruncateTableResponse;
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UnassignRegionRequest;
 import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UnassignRegionResponse;
-import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UpdateMasterConfigurationResponse;
 import org.apache.hadoop.hbase.protobuf.generated.RegionServerStatusProtos.GetLastFlushedSequenceIdRequest;
 import org.apache.hadoop.hbase.protobuf.generated.RegionServerStatusProtos.GetLastFlushedSequenceIdResponse;
 import org.apache.hadoop.hbase.protobuf.generated.RegionServerStatusProtos.RegionServerReportRequest;
@@ -266,14 +265,6 @@ public class MasterRpcServices extends RSRpcServices
     return bssi;
   }
 
-  @Override
-  public UpdateMasterConfigurationResponse updateMasterConfiguration(RpcController controller,
-      org.apache.hadoop.hbase.protobuf.generated.MasterProtos.UpdateMasterConfigurationRequest
-      request) throws ServiceException {
-    master.updateConfiguration();
-    return UpdateMasterConfigurationResponse.getDefaultInstance();
-  }
-
   @Override
   public GetLastFlushedSequenceIdResponse getLastFlushedSequenceId(RpcController controller,
       GetLastFlushedSequenceIdRequest request) throws ServiceException {

hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java

@@ -135,19 +135,15 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
 
     numRegionLoadsToRemember = conf.getInt(KEEP_REGION_LOADS, numRegionLoadsToRemember);
 
-    if (localityCandidateGenerator == null) {
+    localityCandidateGenerator = new LocalityBasedCandidateGenerator(services);
-      localityCandidateGenerator = new LocalityBasedCandidateGenerator(services);
-    }
     localityCost = new LocalityCostFunction(conf, services);
 
-    if (candidateGenerators == null) {
+    candidateGenerators = new CandidateGenerator[] {
-      candidateGenerators = new CandidateGenerator[] {
+      new RandomCandidateGenerator(),
-          new RandomCandidateGenerator(),
+      new LoadCandidateGenerator(),
-          new LoadCandidateGenerator(),
+      localityCandidateGenerator,
-          localityCandidateGenerator,
+      new RegionReplicaRackCandidateGenerator(),
-          new RegionReplicaRackCandidateGenerator(),
+    };
-      };
-    }
 
     regionLoadFunctions = new CostFromRegionLoadFunction[] {
       new ReadRequestCostFunction(conf),

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestUpdateConfiguration.java

@@ -18,17 +18,10 @@
  */
 package org.apache.hadoop.hbase.client;
 
-import static org.junit.Assert.assertEquals;
-
 import java.io.IOException;
-import java.nio.file.FileSystems;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.StandardCopyOption;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
@@ -53,24 +46,4 @@ public class TestUpdateConfiguration {
     ServerName server = TEST_UTIL.getHBaseCluster().getRegionServer(0).getServerName();
     admin.updateConfiguration(server);
   }
-
-  @Test
-  public void testMasterOnlineConfigChange() throws IOException {
-    LOG.debug("Starting the test");
-    Path cnfPath = FileSystems.getDefault().getPath("target/test-classes/hbase-site.xml");
-    Path cnf2Path = FileSystems.getDefault().getPath("target/test-classes/hbase-site2.xml");
-    Path cnf3Path = FileSystems.getDefault().getPath("target/test-classes/hbase-site3.xml");
-    // make a backup of hbase-site.xml
-    Files.copy(cnfPath, cnf3Path, StandardCopyOption.REPLACE_EXISTING);
-    // update hbase-site.xml by overwriting it
-    Files.copy(cnf2Path, cnfPath, StandardCopyOption.REPLACE_EXISTING);
-
-    Admin admin = TEST_UTIL.getHBaseAdmin();
-    admin.updateMasterConfiguration();
-    Configuration conf = TEST_UTIL.getMiniHBaseCluster().getMaster().getConfiguration();
-    int custom = conf.getInt("hbase.custom.config", 0);
-    assertEquals(custom, 1000);
-    // restore hbase-site.xml
-    Files.copy(cnf3Path, cnfPath, StandardCopyOption.REPLACE_EXISTING);
-  }
 }

hbase-server/src/test/resources/hbase-site2.xml

@@ -1,154 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet type="text/xsl" href="configuration.xsl"?>
-<!--
-/**
- *
- * 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.
- */
--->
-<configuration>
-  <property>
-    <name>hbase.custom.config</name>
-    <value>1000</value>
-  </property>
-  <property>
-    <name>hbase.regionserver.msginterval</name>
-    <value>1000</value>
-    <description>Interval between messages from the RegionServer to HMaster
-    in milliseconds.  Default is 15. Set this value low if you want unit
-    tests to be responsive.
-    </description>
-  </property>
-  <property>
-    <name>hbase.defaults.for.version.skip</name>
-    <value>true</value>
-  </property>
-  <property>
-    <name>hbase.server.thread.wakefrequency</name>
-    <value>1000</value>
-    <description>Time to sleep in between searches for work (in milliseconds).
-    Used as sleep interval by service threads such as hbase:meta scanner and log roller.
-    </description>
-  </property>
-  <property>
-    <name>hbase.master.event.waiting.time</name>
-    <value>50</value>
-    <description>Time to sleep between checks to see if a table event took place.
-    </description>
-  </property>
-  <property>
-    <name>hbase.regionserver.handler.count</name>
-    <value>5</value>
-  </property>
-  <property>
-    <name>hbase.master.info.port</name>
-    <value>-1</value>
-    <description>The port for the hbase master web UI
-    Set to -1 if you do not want the info server to run.
-    </description>
-  </property>
-  <property>
-    <name>hbase.master.port</name>
-    <value>0</value>
-    <description>Always have masters and regionservers come up on port '0' so we don't clash over
-      default ports.
-    </description>
-  </property>
-  <property>
-    <name>hbase.regionserver.port</name>
-    <value>0</value>
-    <description>Always have masters and regionservers come up on port '0' so we don't clash over
-      default ports.
-    </description>
-  </property>
-  <property>
-    <name>hbase.ipc.client.fallback-to-simple-auth-allowed</name>
-    <value>true</value>
-  </property>
-
-  <property>
-    <name>hbase.regionserver.info.port</name>
-    <value>-1</value>
-    <description>The port for the hbase regionserver web UI
-    Set to -1 if you do not want the info server to run.
-    </description>
-  </property>
-  <property>
-    <name>hbase.regionserver.info.port.auto</name>
-    <value>true</value>
-    <description>Info server auto port bind. Enables automatic port
-    search if hbase.regionserver.info.port is already in use.
-    Enabled for testing to run multiple tests on one machine.
-    </description>
-  </property>
-  <property>
-    <name>hbase.master.lease.thread.wakefrequency</name>
-    <value>3000</value>
-    <description>The interval between checks for expired region server leases.
-    This value has been reduced due to the other reduced values above so that
-    the master will notice a dead region server sooner. The default is 15 seconds.
-    </description>
-  </property>
-  <property>
-    <name>hbase.regionserver.safemode</name>
-    <value>false</value>
-    <description>
-    Turn on/off safe mode in region server. Always on for production, always off
-    for tests.
-    </description>
-  </property>
-  <property>
-    <name>hbase.hregion.max.filesize</name>
-    <value>67108864</value>
-    <description>
-    Maximum desired file size for an HRegion.  If filesize exceeds
-    value + (value / 2), the HRegion is split in two.  Default: 256M.
-
-    Keep the maximum filesize small so we split more often in tests.
-    </description>
-  </property>
-  <property>
-    <name>hadoop.log.dir</name>
-    <value>${user.dir}/../logs</value>
-  </property>
-  <property>
-    <name>hbase.zookeeper.property.clientPort</name>
-    <value>21818</value>
-    <description>Property from ZooKeeper's config zoo.cfg.
-    The port at which the clients will connect.
-    </description>
-  </property>
-  <property>
-    <name>hbase.defaults.for.version.skip</name>
-    <value>true</value>
-    <description>
-    Set to true to skip the 'hbase.defaults.for.version'.
-    Setting this to true can be useful in contexts other than
-    the other side of a maven generation; i.e. running in an
-    ide.  You'll want to set this boolean to true to avoid
-    seeing the RuntimException complaint: "hbase-default.xml file
-    seems to be for and old version of HBase (@@@VERSION@@@), this
-    version is X.X.X-SNAPSHOT"
-    </description>
-  </property>
-  <property>
-    <name>hbase.table.sanity.checks</name>
-    <value>false</value>
-    <description>Skip sanity checks in tests
-    </description>
-  </property>
-</configuration>