MAPREDUCE-6761. Regression when handling providers - invalid configuration ServiceConfiguration causes Cluster initialization failure. Contributed by Peter Vary

(cherry picked from commit 6fa9bf4407)

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Cluster.java

@@ -24,8 +24,10 @@ import java.net.InetSocketAddress;
 import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.classification.InterfaceAudience;
@@ -64,7 +66,8 @@ public class Cluster {
   private Path jobHistoryDir = null;
   private static final Log LOG = LogFactory.getLog(Cluster.class);
 
-  private static ServiceLoader<ClientProtocolProvider> frameworkLoader =
+  @VisibleForTesting
+  static Iterable<ClientProtocolProvider> frameworkLoader =
       ServiceLoader.load(ClientProtocolProvider.class);
   private volatile List<ClientProtocolProvider> providerList = null;
 
@@ -74,9 +77,16 @@ public class Cluster {
         if (providerList == null) {
           List<ClientProtocolProvider> localProviderList =
               new ArrayList<ClientProtocolProvider>();
+          try {
             for (ClientProtocolProvider provider : frameworkLoader) {
               localProviderList.add(provider);
             }
+          } catch(ServiceConfigurationError e) {
+            LOG.info("Failed to instantiate ClientProtocolProvider, please "
+                         + "check the /META-INF/services/org.apache."
+                         + "hadoop.mapreduce.protocol.ClientProtocolProvider "
+                         + "files on the classpath", e);
+          }
           providerList = localProviderList;
         }
       }

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestCluster.java

@@ -0,0 +1,81 @@
+/**
+ * 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.hadoop.mapreduce;
+
+import org.apache.hadoop.conf.Configuration;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.times;
+import static org.junit.Assert.assertNotNull;
+
+import org.apache.hadoop.mapreduce.protocol.ClientProtocol;
+import org.apache.hadoop.mapreduce.protocol.ClientProtocolProvider;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Iterator;
+import java.util.ServiceConfigurationError;
+
+/**
+ * Testing the Cluster initialization.
+ */
+public class TestCluster {
+  @Test
+  @SuppressWarnings("unchecked")
+  public void testProtocolProviderCreation() throws Exception {
+    Iterator iterator = mock(Iterator.class);
+    when(iterator.hasNext()).thenReturn(true, true, true, true);
+    when(iterator.next()).thenReturn(getClientProtocolProvider())
+        .thenThrow(new ServiceConfigurationError("Test error"))
+        .thenReturn(getClientProtocolProvider());
+
+    Iterable frameworkLoader = mock(Iterable.class);
+    when(frameworkLoader.iterator()).thenReturn(iterator);
+
+    Cluster.frameworkLoader = frameworkLoader;
+    Cluster testCluster = new Cluster(new Configuration());
+
+    // Check that we get the acceptable client, even after
+    // failure in instantiation.
+    assertNotNull("ClientProtocol is expected", testCluster.getClient());
+    // Check if we do not try to load the providers after a failure.
+    verify(iterator, times(2)).next();
+  }
+
+  public ClientProtocolProvider getClientProtocolProvider() {
+    return new ClientProtocolProvider() {
+      @Override
+      public ClientProtocol create(Configuration conf) throws IOException {
+        return mock(ClientProtocol.class);
+      }
+
+      @Override
+      public ClientProtocol create(InetSocketAddress addr, Configuration
+          conf) throws IOException {
+        return mock(ClientProtocol.class);
+      }
+
+      @Override
+      public void close(ClientProtocol clientProtocol) throws IOException {
+      }
+    };
+  }
+}