This is an automated email from the ASF dual-hosted git repository. tgraves pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push: new 423ba5a [SPARK-32916][SHUFFLE][TEST-MAVEN][TEST-HADOOP2.7] Remove the newly added YarnShuffleServiceSuite.java 423ba5a is described below commit 423ba5a16038c1cb28d0973e18518645e69d5ff1 Author: Chandni Singh <singh.chan...@gmail.com> AuthorDate: Fri Nov 13 16:16:23 2020 -0600 [SPARK-32916][SHUFFLE][TEST-MAVEN][TEST-HADOOP2.7] Remove the newly added YarnShuffleServiceSuite.java ### What changes were proposed in this pull request? This is a follow-up fix for the failing tests in `YarnShuffleServiceSuite.java`. This java class was introduced in https://github.com/apache/spark/pull/30062. The tests in the class fail when run with hadoop-2.7 profile: ``` [ERROR] testCreateDefaultMergedShuffleFileManagerInstance(org.apache.spark.network.yarn.YarnShuffleServiceSuite) Time elapsed: 0.627 s <<< ERROR! java.lang.NoClassDefFoundError: org/apache/commons/logging/LogFactory at org.apache.spark.network.yarn.YarnShuffleServiceSuite.testCreateDefaultMergedShuffleFileManagerInstance(YarnShuffleServiceSuite.java:37) Caused by: java.lang.ClassNotFoundException: org.apache.commons.logging.LogFactory at org.apache.spark.network.yarn.YarnShuffleServiceSuite.testCreateDefaultMergedShuffleFileManagerInstance(YarnShuffleServiceSuite.java:37) [ERROR] testCreateRemoteBlockPushResolverInstance(org.apache.spark.network.yarn.YarnShuffleServiceSuite) Time elapsed: 0 s <<< ERROR! java.lang.NoClassDefFoundError: Could not initialize class org.apache.spark.network.yarn.YarnShuffleService at org.apache.spark.network.yarn.YarnShuffleServiceSuite.testCreateRemoteBlockPushResolverInstance(YarnShuffleServiceSuite.java:47) [ERROR] testInvalidClassNameOfMergeManagerWillUseNoOpInstance(org.apache.spark.network.yarn.YarnShuffleServiceSuite) Time elapsed: 0.001 s <<< ERROR! java.lang.NoClassDefFoundError: Could not initialize class org.apache.spark.network.yarn.YarnShuffleService at org.apache.spark.network.yarn.YarnShuffleServiceSuite.testInvalidClassNameOfMergeManagerWillUseNoOpInstance(YarnShuffleServiceSuite.java:57) ``` A test suit for `YarnShuffleService` did exist here: `resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala` I missed this when I created `common/network-yarn/src/test/java/org/apache/spark/network/yarn/YarnShuffleServiceSuite.java`. Moving all the new tests to the earlier test suite fixes the failures with hadoop-2.7 even though why this happened is not clear. ### Why are the changes needed? The newly added tests are failing when run with hadoop profile 2.7 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Ran the unit tests with the default profile as well as hadoop 2.7 profile. `build/mvn test -Dtest=none -DwildcardSuites=org.apache.spark.network.yarn.YarnShuffleServiceSuite -Phadoop-2.7 -Pyarn` ``` Run starting. Expected test count is: 11 YarnShuffleServiceSuite: - executor state kept across NM restart - removed applications should not be in registered executor file - shuffle service should be robust to corrupt registered executor file - get correct recovery path - moving recovery file from NM local dir to recovery path - service throws error if cannot start - recovery db should not be created if NM recovery is not enabled - SPARK-31646: metrics should be registered into Node Manager's metrics system - create default merged shuffle file manager instance - create remote block push resolver instance - invalid class name of merge manager will use noop instance Run completed in 2 seconds, 572 milliseconds. Total number of tests run: 11 Suites: completed 2, aborted 0 Tests: succeeded 11, failed 0, canceled 0, ignored 0, pending 0 All tests passed. ``` Closes #30349 from otterc/SPARK-32916-followup. Authored-by: Chandni Singh <singh.chan...@gmail.com> Signed-off-by: Thomas Graves <tgra...@apache.org> --- .../network/yarn/YarnShuffleServiceSuite.java | 61 ---------------------- .../network/yarn/YarnShuffleServiceSuite.scala | 27 +++++++++- 2 files changed, 26 insertions(+), 62 deletions(-) diff --git a/common/network-yarn/src/test/java/org/apache/spark/network/yarn/YarnShuffleServiceSuite.java b/common/network-yarn/src/test/java/org/apache/spark/network/yarn/YarnShuffleServiceSuite.java deleted file mode 100644 index 09bc4d8..0000000 --- a/common/network-yarn/src/test/java/org/apache/spark/network/yarn/YarnShuffleServiceSuite.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * 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.spark.network.yarn; - -import org.junit.Test; - -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; - -import org.apache.spark.network.shuffle.ExternalBlockHandler; -import org.apache.spark.network.shuffle.MergedShuffleFileManager; -import org.apache.spark.network.shuffle.RemoteBlockPushResolver; -import org.apache.spark.network.util.TransportConf; - -public class YarnShuffleServiceSuite { - - @Test - public void testCreateDefaultMergedShuffleFileManagerInstance() { - TransportConf mockConf = mock(TransportConf.class); - when(mockConf.mergedShuffleFileManagerImpl()).thenReturn( - "org.apache.spark.network.shuffle.ExternalBlockHandler$NoOpMergedShuffleFileManager"); - MergedShuffleFileManager mergeMgr = YarnShuffleService.newMergedShuffleFileManagerInstance( - mockConf); - assertTrue(mergeMgr instanceof ExternalBlockHandler.NoOpMergedShuffleFileManager); - } - - @Test - public void testCreateRemoteBlockPushResolverInstance() { - TransportConf mockConf = mock(TransportConf.class); - when(mockConf.mergedShuffleFileManagerImpl()).thenReturn( - "org.apache.spark.network.shuffle.RemoteBlockPushResolver"); - MergedShuffleFileManager mergeMgr = YarnShuffleService.newMergedShuffleFileManagerInstance( - mockConf); - assertTrue(mergeMgr instanceof RemoteBlockPushResolver); - } - - @Test - public void testInvalidClassNameOfMergeManagerWillUseNoOpInstance() { - TransportConf mockConf = mock(TransportConf.class); - when(mockConf.mergedShuffleFileManagerImpl()).thenReturn( - "org.apache.spark.network.shuffle.NotExistent"); - MergedShuffleFileManager mergeMgr = YarnShuffleService.newMergedShuffleFileManagerInstance( - mockConf); - assertTrue(mergeMgr instanceof ExternalBlockHandler.NoOpMergedShuffleFileManager); - } -} diff --git a/resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala b/resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala index a6a302a..c2bdd97 100644 --- a/resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala +++ b/resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala @@ -34,6 +34,7 @@ import org.apache.hadoop.service.ServiceStateException import org.apache.hadoop.yarn.api.records.ApplicationId import org.apache.hadoop.yarn.conf.YarnConfiguration import org.apache.hadoop.yarn.server.api.{ApplicationInitializationContext, ApplicationTerminationContext} +import org.mockito.Mockito.{mock, when} import org.scalatest.BeforeAndAfterEach import org.scalatest.concurrent.Eventually._ import org.scalatest.matchers.must.Matchers @@ -42,8 +43,9 @@ import org.scalatest.matchers.should.Matchers._ import org.apache.spark.SecurityManager import org.apache.spark.SparkFunSuite import org.apache.spark.internal.config._ -import org.apache.spark.network.shuffle.ShuffleTestAccessor +import org.apache.spark.network.shuffle.{ExternalBlockHandler, RemoteBlockPushResolver, ShuffleTestAccessor} import org.apache.spark.network.shuffle.protocol.ExecutorShuffleInfo +import org.apache.spark.network.util.TransportConf import org.apache.spark.util.Utils class YarnShuffleServiceSuite extends SparkFunSuite with Matchers with BeforeAndAfterEach { @@ -411,4 +413,27 @@ class YarnShuffleServiceSuite extends SparkFunSuite with Matchers with BeforeAnd )) } + test("create default merged shuffle file manager instance") { + val mockConf = mock(classOf[TransportConf]) + when(mockConf.mergedShuffleFileManagerImpl).thenReturn( + "org.apache.spark.network.shuffle.ExternalBlockHandler$NoOpMergedShuffleFileManager") + val mergeMgr = YarnShuffleService.newMergedShuffleFileManagerInstance(mockConf) + assert(mergeMgr.isInstanceOf[ExternalBlockHandler.NoOpMergedShuffleFileManager]) + } + + test("create remote block push resolver instance") { + val mockConf = mock(classOf[TransportConf]) + when(mockConf.mergedShuffleFileManagerImpl).thenReturn( + "org.apache.spark.network.shuffle.RemoteBlockPushResolver") + val mergeMgr = YarnShuffleService.newMergedShuffleFileManagerInstance(mockConf) + assert(mergeMgr.isInstanceOf[RemoteBlockPushResolver]) + } + + test("invalid class name of merge manager will use noop instance") { + val mockConf = mock(classOf[TransportConf]) + when(mockConf.mergedShuffleFileManagerImpl).thenReturn( + "org.apache.spark.network.shuffle.NotExistent") + val mergeMgr = YarnShuffleService.newMergedShuffleFileManagerInstance(mockConf) + assert(mergeMgr.isInstanceOf[ExternalBlockHandler.NoOpMergedShuffleFileManager]) + } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org