From b3a0e68d477bfc76c69c40dd3ad89a4800a7a8f7 Mon Sep 17 00:00:00 2001 From: Mark Bathori Date: Mon, 31 Jan 2022 17:22:12 +0100 Subject: [PATCH 1/9] TEZ-4380: TestSecureShuffle fails on branch-0.9 --- .../src/test/java/org/apache/tez/test/TestSecureShuffle.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java index 00196a20e9..a25bbe99fe 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java @@ -113,6 +113,7 @@ public static Collection getParameters() { public static void setupDFSCluster() throws Exception { conf = new Configuration(); conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_EDITS_NOEDITLOGCHANNELFLUSH, false); + conf.setBoolean("fs.hdfs.impl.disable.cache", true); EditLogFileOutputStream.setShouldSkipFsyncForTesting(true); conf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR); miniDFSCluster = @@ -300,7 +301,8 @@ public static X509Certificate generateCertificate(String dn, KeyPair pair, int d String hostAddress = InetAddress.getLocalHost().getHostAddress(); certGen.addExtension(X509Extensions.SubjectAlternativeName, false, - new GeneralNames(new GeneralName(GeneralName.iPAddress, hostAddress))); + new GeneralNames(new GeneralName[] { new GeneralName(GeneralName.iPAddress, hostAddress), + new GeneralName(GeneralName.dNSName, "localhost") })); X500Principal dnName = new X500Principal(dn); From b3f9f0aad2af0c8dc0769ff13bba6bc7915e9556 Mon Sep 17 00:00:00 2001 From: Mark Bathori Date: Mon, 31 Jan 2022 22:07:57 +0100 Subject: [PATCH 2/9] Remove conf set --- .../src/test/java/org/apache/tez/test/TestSecureShuffle.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java index a25bbe99fe..8a013e20ee 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java @@ -113,7 +113,6 @@ public static Collection getParameters() { public static void setupDFSCluster() throws Exception { conf = new Configuration(); conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_EDITS_NOEDITLOGCHANNELFLUSH, false); - conf.setBoolean("fs.hdfs.impl.disable.cache", true); EditLogFileOutputStream.setShouldSkipFsyncForTesting(true); conf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR); miniDFSCluster = @@ -303,7 +302,6 @@ public static X509Certificate generateCertificate(String dn, KeyPair pair, int d certGen.addExtension(X509Extensions.SubjectAlternativeName, false, new GeneralNames(new GeneralName[] { new GeneralName(GeneralName.iPAddress, hostAddress), new GeneralName(GeneralName.dNSName, "localhost") })); - X500Principal dnName = new X500Principal(dn); certGen.setSerialNumber(sn); From 853854061f330c4518c1ca2eebfa87bf018028ac Mon Sep 17 00:00:00 2001 From: Laszlo Bodor Date: Mon, 28 Sep 2020 08:28:53 +0200 Subject: [PATCH 3/9] Map task should be blamed earlier for local fetch failures (Laszlo Bdoor reviewed by Rajesh Balamohan) --- .../api/events/InputReadErrorEvent.java | 27 ++- .../shuffle/api/ShuffleHandlerError.java | 27 +++ .../common/shuffle/api/package-info.java | 22 +++ tez-api/src/main/proto/Events.proto | 2 + .../tez/dag/app/dag/impl/TaskAttemptImpl.java | 9 +- .../tez/dag/app/dag/impl/TestTaskAttempt.java | 49 ++++- .../tez/auxservices/ShuffleHandler.java | 53 +++++- .../tez/auxservices/TestShuffleHandler.java | 100 +++++++++- .../apache/tez/runtime/api/impl/TezEvent.java | 9 +- .../library/common/shuffle/Fetcher.java | 63 ++++--- .../common/shuffle/FetcherCallback.java | 3 +- .../shuffle/InputAttemptFetchFailure.java | 115 ++++++++++++ .../common/shuffle/impl/ShuffleManager.java | 16 +- .../orderedgrouped/FetcherOrderedGrouped.java | 74 +++++--- .../shuffle/orderedgrouped/ShuffleHeader.java | 7 + .../orderedgrouped/ShuffleScheduler.java | 45 ++--- .../library/common/shuffle/TestFetcher.java | 54 +++++- .../shuffle/impl/TestShuffleManager.java | 3 +- .../shuffle/orderedgrouped/TestFetcher.java | 66 +++++-- .../orderedgrouped/TestShuffleScheduler.java | 174 +++++++++--------- .../library/testutils/RuntimeTestUtils.java | 44 +++++ 21 files changed, 752 insertions(+), 210 deletions(-) create mode 100644 tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/ShuffleHandlerError.java create mode 100644 tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/package-info.java create mode 100644 tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java create mode 100644 tez-runtime-library/src/test/java/org/apache/tez/runtime/library/testutils/RuntimeTestUtils.java diff --git a/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java b/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java index 7d2e0d25a8..3fa6551212 100644 --- a/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java +++ b/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java @@ -44,17 +44,30 @@ public final class InputReadErrorEvent extends Event { */ private final int version; + /** + * Whether this input read error is caused while fetching local file. + */ + private final boolean isLocalFetch; + + /** + * Whether this input read error is caused because the fetcher detected a fatal, unrecoverable, + * local file read issue from the shuffle handler. + */ + private final boolean isDiskErrorAtSource; + private InputReadErrorEvent(String diagnostics, int index, - int version) { + int version, boolean isLocalFetch, boolean isDiskErrorAtSource) { super(); this.diagnostics = diagnostics; this.index = index; this.version = version; + this.isLocalFetch = isLocalFetch; + this.isDiskErrorAtSource = isDiskErrorAtSource; } public static InputReadErrorEvent create(String diagnostics, int index, - int version) { - return new InputReadErrorEvent(diagnostics, index, version); + int version, boolean isLocalFetch, boolean isDiskErrorAtSource) { + return new InputReadErrorEvent(diagnostics, index, version, isLocalFetch, isDiskErrorAtSource); } public String getDiagnostics() { @@ -69,4 +82,12 @@ public int getVersion() { return version; } + public boolean isLocalFetch() { + return isLocalFetch; + } + + public boolean isDiskErrorAtSource() { + return isDiskErrorAtSource; + } + } diff --git a/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/ShuffleHandlerError.java b/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/ShuffleHandlerError.java new file mode 100644 index 0000000000..09137de673 --- /dev/null +++ b/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/ShuffleHandlerError.java @@ -0,0 +1,27 @@ +/** +* 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.tez.runtime.library.common.shuffle.api; + +/** + * ShuffleHandlerError enum encapsulates possible error messages that can be propagated from + * ShuffleHandler to fetchers. Depending on the message, fetchers can make better decisions, or give + * AM a hint in order to let it make better decisions in case of shuffle issues. + */ +public enum ShuffleHandlerError { + DISK_ERROR_EXCEPTION +} diff --git a/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/package-info.java b/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/package-info.java new file mode 100644 index 0000000000..9ad8e61d50 --- /dev/null +++ b/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/package-info.java @@ -0,0 +1,22 @@ +/** + * 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. + */ + +@Private +package org.apache.tez.runtime.library.common.shuffle.api; + +import org.apache.hadoop.classification.InterfaceAudience.Private; \ No newline at end of file diff --git a/tez-api/src/main/proto/Events.proto b/tez-api/src/main/proto/Events.proto index 71235004da..e041c33f60 100644 --- a/tez-api/src/main/proto/Events.proto +++ b/tez-api/src/main/proto/Events.proto @@ -39,6 +39,8 @@ message InputReadErrorEventProto { optional int32 index = 1; optional string diagnostics = 2; optional int32 version = 3; + optional bool is_local_fetch = 4; + optional bool is_disk_error_at_source = 5; } message InputFailedEventProto { diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java b/tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java index 5a691190e9..5394128044 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java @@ -1769,6 +1769,7 @@ public TaskAttemptStateInternal transition(TaskAttemptImpl attempt, + " at inputIndex " + failedInputIndexOnDestTa); long time = attempt.clock.getTime(); Long firstErrReportTime = attempt.uniquefailedOutputReports.get(failedDestTaId); + if (firstErrReportTime == null) { attempt.uniquefailedOutputReports.put(failedDestTaId, time); firstErrReportTime = time; @@ -1795,7 +1796,8 @@ public TaskAttemptStateInternal transition(TaskAttemptImpl attempt, // If needed we can launch a background task without failing this task // to generate a copy of the output just in case. // If needed we can consider only running consumer tasks - if (!crossTimeDeadline && withinFailureFractionLimits && withinOutputFailureLimits) { + if (!crossTimeDeadline && withinFailureFractionLimits && withinOutputFailureLimits + && !(readErrorEvent.isLocalFetch() || readErrorEvent.isDiskErrorAtSource())) { return attempt.getInternalState(); } String message = attempt.getID() + " being failed for too many output errors. " @@ -1806,7 +1808,10 @@ public TaskAttemptStateInternal transition(TaskAttemptImpl attempt, + ", MAX_ALLOWED_OUTPUT_FAILURES=" + maxAllowedOutputFailures + ", MAX_ALLOWED_TIME_FOR_TASK_READ_ERROR_SEC=" + maxAllowedTimeForTaskReadErrorSec - + ", readErrorTimespan=" + readErrorTimespanSec; + + ", readErrorTimespan=" + readErrorTimespanSec + + ", isLocalFetch=" + readErrorEvent.isLocalFetch() + + ", isDiskErrorAtSource=" + readErrorEvent.isDiskErrorAtSource(); + LOG.info(message); attempt.addDiagnosticInfo(message); // send input failed event diff --git a/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java b/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java index 41cce3b60e..6862bec2ee 100644 --- a/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java +++ b/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java @@ -19,7 +19,6 @@ package org.apache.tez.dag.app.dag.impl; import org.apache.tez.dag.app.MockClock; -import org.apache.tez.dag.app.rm.AMSchedulerEventTAStateUpdated; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; @@ -83,6 +82,7 @@ import org.apache.tez.dag.app.TaskCommunicatorWrapper; import org.apache.tez.dag.app.TaskHeartbeatHandler; import org.apache.tez.dag.app.dag.TaskAttemptStateInternal; +import org.apache.tez.dag.app.dag.DAG; import org.apache.tez.dag.app.dag.Task; import org.apache.tez.dag.app.dag.Vertex; import org.apache.tez.dag.app.dag.event.DAGEvent; @@ -127,6 +127,7 @@ import org.junit.BeforeClass; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -2154,6 +2155,52 @@ taListener, taskConf, new SystemClock(), Assert.assertEquals(1, taImpl.taskAttemptFinishedEventLogged); } + @Test + public void testMapTaskIsBlamedImmediatelyOnLocalFetchFailure() throws ServicePluginException { + // local fetch failure or disk read error at source -> turn source attempt to FAIL_IN_PROGRESS + testMapTaskFailingForFetchFailureType(true, true, TaskAttemptStateInternal.FAIL_IN_PROGRESS); + testMapTaskFailingForFetchFailureType(true, false, TaskAttemptStateInternal.FAIL_IN_PROGRESS); + testMapTaskFailingForFetchFailureType(false, true, TaskAttemptStateInternal.FAIL_IN_PROGRESS); + + // remote fetch failure -> won't change current state + testMapTaskFailingForFetchFailureType(false, false, TaskAttemptStateInternal.NEW); + } + + private void testMapTaskFailingForFetchFailureType(boolean isLocalFetch, + boolean isDiskErrorAtSource, TaskAttemptStateInternal expectedState) { + EventHandler eventHandler = mock(EventHandler.class); + TezTaskID taskID = + TezTaskID.getInstance(TezVertexID.getInstance(TezDAGID.getInstance("1", 1, 1), 1), 1); + TaskAttemptImpl sourceAttempt = new MockTaskAttemptImpl(taskID, 1, eventHandler, null, + new Configuration(), SystemClock.getInstance(), mock(TaskHeartbeatHandler.class), appCtx, + false, null, null, false); + + // the original read error event, sent by reducer task + InputReadErrorEvent inputReadErrorEvent = + InputReadErrorEvent.create("", 0, 1, 1, isLocalFetch, isDiskErrorAtSource); + TezTaskAttemptID destTaskAttemptId = mock(TezTaskAttemptID.class); + when(destTaskAttemptId.getTaskID()).thenReturn(mock(TezTaskID.class)); + when(destTaskAttemptId.getTaskID().getVertexID()).thenReturn(mock(TezVertexID.class)); + when(appCtx.getCurrentDAG()).thenReturn(mock(DAG.class)); + when(appCtx.getCurrentDAG().getVertex(Mockito.any(TezVertexID.class))) + .thenReturn(mock(Vertex.class)); + when(appCtx.getCurrentDAG().getVertex(Mockito.any(TezVertexID.class)).getRunningTasks()) + .thenReturn(100); + + EventMetaData mockMeta = mock(EventMetaData.class); + when(mockMeta.getTaskAttemptID()).thenReturn(destTaskAttemptId); + TezEvent tezEvent = new TezEvent(inputReadErrorEvent, mockMeta); + + // the event is propagated to map task's event handler + TaskAttemptEventOutputFailed outputFailedEvent = + new TaskAttemptEventOutputFailed(sourceAttempt.getID(), tezEvent, 11); + + Assert.assertEquals(TaskAttemptStateInternal.NEW, sourceAttempt.getInternalState()); + TaskAttemptStateInternal resultState = new TaskAttemptImpl.OutputReportedFailedTransition() + .transition(sourceAttempt, outputFailedEvent); + Assert.assertEquals(expectedState, resultState); + } + private Event verifyEventType(List events, Class eventClass, int expectedOccurences) { int count = 0; diff --git a/tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java b/tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java index 1bdddde529..fb28a0f4b1 100644 --- a/tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java +++ b/tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java @@ -71,6 +71,7 @@ import org.apache.tez.common.security.JobTokenSecretManager; import org.apache.tez.runtime.library.common.Constants; import org.apache.tez.runtime.library.common.security.SecureShuffleUtils; +import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.ShuffleHeader; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.annotation.Metric; @@ -84,6 +85,7 @@ import org.apache.hadoop.security.token.Token; import org.apache.tez.runtime.library.common.sort.impl.TezIndexRecord; import org.apache.hadoop.util.Shell; +import org.apache.hadoop.util.DiskChecker.DiskErrorException; import org.apache.hadoop.yarn.api.records.ApplicationId; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.proto.YarnServerCommonProtos.VersionProto; @@ -644,6 +646,10 @@ protected Shuffle getShuffle(Configuration conf) { return new Shuffle(conf); } + protected JobTokenSecretManager getSecretManager() { + return secretManager; + } + private void recoverState(Configuration conf) throws IOException { Path recoveryRoot = getRecoveryPath(); if (recoveryRoot != null) { @@ -763,7 +769,7 @@ private void checkVersion() throws IOException { private void addJobToken(JobID jobId, String user, Token jobToken) { userRsrc.put(jobId.toString(), user); - secretManager.addTokenForJob(jobId.toString(), jobToken); + getSecretManager().addTokenForJob(jobId.toString(), jobToken); LOG.info("Added token for " + jobId.toString()); } @@ -809,7 +815,7 @@ private void recordJobShuffleInfo(JobID jobId, String user, private void removeJobShuffleInfo(JobID jobId) throws IOException { String jobIdStr = jobId.toString(); - secretManager.removeTokenForJob(jobIdStr); + getSecretManager().removeTokenForJob(jobIdStr); userRsrc.remove(jobIdStr); if (stateDb != null) { try { @@ -1074,11 +1080,19 @@ private void handleRequest(ChannelHandlerContext ctx, FullHttpRequest request) try { populateHeaders(mapIds, jobId, dagId, user, reduceRange, response, keepAliveParam, mapOutputInfoMap); - } catch(IOException e) { + } catch (DiskErrorException e) { // fatal error: fetcher should be aware of that + LOG.error("Shuffle error in populating headers (fatal: DiskErrorException):", e); + String errorMessage = getErrorMessage(e); + // custom message, might be noticed by fetchers + // it should reuse the current response object, as headers have been already set for it + sendFakeShuffleHeaderWithError(ctx, + ShuffleHandlerError.DISK_ERROR_EXCEPTION + ": " + errorMessage, response); + return; + } catch (IOException e) { ch.write(response); LOG.error("Shuffle error in populating headers :", e); String errorMessage = getErrorMessage(e); - sendError(ctx,errorMessage , INTERNAL_SERVER_ERROR); + sendError(ctx, errorMessage, INTERNAL_SERVER_ERROR); return; } ch.write(response); @@ -1329,7 +1343,7 @@ public void finish() { protected void verifyRequest(String appid, ChannelHandlerContext ctx, HttpRequest request, HttpResponse response, URL requestUri) throws IOException { - SecretKey tokenSecret = secretManager.retrieveTokenSecret(appid); + SecretKey tokenSecret = getSecretManager().retrieveTokenSecret(appid); if (null == tokenSecret) { LOG.info("Request for unknown token " + appid); throw new IOException("could not find jobid"); @@ -1425,25 +1439,46 @@ protected ChannelFuture sendMapOutput(ChannelHandlerContext ctx, Channel ch, return writeFuture; } - protected void sendError(ChannelHandlerContext ctx, - HttpResponseStatus status) { + protected void sendError(ChannelHandlerContext ctx, HttpResponseStatus status) { sendError(ctx, "", status); } protected void sendError(ChannelHandlerContext ctx, String message, HttpResponseStatus status) { FullHttpResponse response = new DefaultFullHttpResponse(HTTP_1_1, status); + sendError(ctx, message, response); + response.release(); + } + protected void sendError(ChannelHandlerContext ctx, String message, FullHttpResponse response) { + sendError(ctx, Unpooled.copiedBuffer(message, CharsetUtil.UTF_8), response); + } + + private void sendFakeShuffleHeaderWithError(ChannelHandlerContext ctx, String message, + HttpResponse response) throws IOException { + FullHttpResponse fullResponse = + new DefaultFullHttpResponse(response.getProtocolVersion(), response.getStatus()); + fullResponse.headers().set(response.headers()); + + ShuffleHeader header = new ShuffleHeader(message, -1, -1, -1); + DataOutputBuffer out = new DataOutputBuffer(); + header.write(out); + + sendError(ctx, wrappedBuffer(out.getData(), 0, out.getLength()), fullResponse); + fullResponse.release(); + } + + protected void sendError(ChannelHandlerContext ctx, ByteBuf content, + FullHttpResponse response) { response.headers().set(CONTENT_TYPE, "text/plain; charset=UTF-8"); // Put shuffle version into http header response.headers().set(ShuffleHeader.HTTP_HEADER_NAME, ShuffleHeader.DEFAULT_HTTP_HEADER_NAME); response.headers().set(ShuffleHeader.HTTP_HEADER_VERSION, ShuffleHeader.DEFAULT_HTTP_HEADER_VERSION); - response.content().writeBytes(Unpooled.copiedBuffer(message, CharsetUtil.UTF_8)); + response.content().writeBytes(content); // Close the connection as soon as the error message is sent. ctx.channel().writeAndFlush(response).addListener(ChannelFutureListener.CLOSE); - response.release(); } @Override diff --git a/tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java b/tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java index 2bf9cb2ad0..e27ff7c4ef 100644 --- a/tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java +++ b/tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java @@ -20,6 +20,7 @@ //import static org.apache.hadoop.test.MetricsAsserts.assertCounter; //import static org.apache.hadoop.test.MetricsAsserts.assertGauge; //import static org.apache.hadoop.test.MetricsAsserts.getMetrics; +import org.apache.hadoop.util.DiskChecker.DiskErrorException; import static org.junit.Assert.assertTrue; import static io.netty.buffer.Unpooled.wrappedBuffer; import static org.junit.Assert.assertEquals; @@ -39,7 +40,6 @@ import java.net.URL; import java.nio.ByteBuffer; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; @@ -48,7 +48,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; -import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.fs.Path; @@ -59,6 +58,10 @@ import org.apache.tez.runtime.library.common.security.SecureShuffleUtils; import org.apache.tez.common.security.JobTokenIdentifier; import org.apache.tez.common.security.JobTokenSecretManager; +import org.apache.tez.http.BaseHttpConnection; +import org.apache.tez.http.HttpConnectionParams; +import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; +import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.ShuffleHeader; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.impl.MetricsSystemImpl; @@ -169,6 +172,52 @@ protected boolean isSocketKeepAlive() { } } + class MockShuffleHandlerWithFatalDiskError extends org.apache.tez.auxservices.ShuffleHandler { + public static final String MESSAGE = + "Could not find application_1234/240/output/attempt_1234_0/file.out.index"; + + private JobTokenSecretManager secretManager = + new JobTokenSecretManager(JobTokenSecretManager.createSecretKey(getSecret().getBytes())); + + protected JobTokenSecretManager getSecretManager(){ + return secretManager; + } + + @Override + protected Shuffle getShuffle(final Configuration conf) { + return new Shuffle(conf) { + @Override + protected void verifyRequest(String appid, ChannelHandlerContext ctx, HttpRequest request, + HttpResponse response, URL requestUri) throws IOException { + super.verifyRequest(appid, ctx, request, response, requestUri); + } + + @Override + protected MapOutputInfo getMapOutputInfo(String dagId, String mapId, Range reduceRange, + String jobId, String user) { + return null; + } + + @Override + protected void populateHeaders(List mapIds, String jobId, String dagId, String user, + Range reduceRange, HttpResponse response, boolean keepAliveParam, + Map infoMap) throws IOException { + throw new DiskErrorException(MESSAGE); + } + + @Override + protected ChannelFuture sendMapOutput(ChannelHandlerContext ctx, Channel ch, String user, + String mapId, Range reduceRange, MapOutputInfo info) throws IOException { + return null; + } + }; + } + + public String getSecret() { + return "secret"; + } + } + /** * Test the validation of ShuffleHandler's meta-data's serialization and * de-serialization. @@ -1310,6 +1359,53 @@ public void testSendMapCount() throws Exception { sh.close(); } + @Test + public void testShuffleHandlerSendsDiskError() throws Exception { + Configuration conf = new Configuration(); + conf.setInt(ShuffleHandler.SHUFFLE_PORT_CONFIG_KEY, 0); + + DataInputStream input = null; + MockShuffleHandlerWithFatalDiskError shuffleHandler = + new MockShuffleHandlerWithFatalDiskError(); + try { + shuffleHandler.init(conf); + shuffleHandler.start(); + + String shuffleBaseURL = "http://127.0.0.1:" + + shuffleHandler.getConfig().get(ShuffleHandler.SHUFFLE_PORT_CONFIG_KEY); + URL url = new URL( + shuffleBaseURL + "/mapOutput?job=job_12345_1&dag=1&reduce=1&map=attempt_12345_1_m_1_0"); + shuffleHandler.secretManager.addTokenForJob("job_12345_1", + new Token<>("id".getBytes(), shuffleHandler.getSecret().getBytes(), null, null)); + + HttpConnectionParams httpConnectionParams = ShuffleUtils.getHttpConnectionParams(conf); + BaseHttpConnection httpConnection = ShuffleUtils.getHttpConnection(true, url, + httpConnectionParams, "testFetcher", shuffleHandler.secretManager); + + boolean connectSucceeded = httpConnection.connect(); + Assert.assertTrue(connectSucceeded); + + input = httpConnection.getInputStream(); + httpConnection.validate(); + + ShuffleHeader header = new ShuffleHeader(); + header.readFields(input); + + // message is encoded in the shuffle header, and can be checked by fetchers + Assert.assertEquals( + ShuffleHandlerError.DISK_ERROR_EXCEPTION + ": " + MockShuffleHandlerWithFatalDiskError.MESSAGE, + header.getMapId()); + Assert.assertEquals(-1, header.getCompressedLength()); + Assert.assertEquals(-1, header.getUncompressedLength()); + Assert.assertEquals(-1, header.getPartition()); + } finally { + if (input != null) { + input.close(); + } + shuffleHandler.close(); + } + } + public ChannelFuture createMockChannelFuture(Channel mockCh, final List listenerList) { final ChannelFuture mockFuture = mock(ChannelFuture.class); diff --git a/tez-runtime-internals/src/main/java/org/apache/tez/runtime/api/impl/TezEvent.java b/tez-runtime-internals/src/main/java/org/apache/tez/runtime/api/impl/TezEvent.java index e7af4a1ebe..ebea9a4f3f 100644 --- a/tez-runtime-internals/src/main/java/org/apache/tez/runtime/api/impl/TezEvent.java +++ b/tez-runtime-internals/src/main/java/org/apache/tez/runtime/api/impl/TezEvent.java @@ -191,6 +191,8 @@ private void serializeEvent(DataOutput out) throws IOException { .setIndex(ideEvt.getIndex()) .setDiagnostics(ideEvt.getDiagnostics()) .setVersion(ideEvt.getVersion()) + .setIsLocalFetch(ideEvt.isLocalFetch()) + .setIsDiskErrorAtSource(ideEvt.isDiskErrorAtSource()) .build(); break; case TASK_ATTEMPT_FAILED_EVENT: @@ -294,10 +296,9 @@ private void deserializeEvent(DataInput in) throws IOException { event = ProtoConverters.convertVertexManagerEventFromProto(vmProto); break; case INPUT_READ_ERROR_EVENT: - InputReadErrorEventProto ideProto = - InputReadErrorEventProto.parseFrom(input); - event = InputReadErrorEvent.create(ideProto.getDiagnostics(), - ideProto.getIndex(), ideProto.getVersion()); + InputReadErrorEventProto ideProto = InputReadErrorEventProto.parseFrom(input); + event = InputReadErrorEvent.create(ideProto.getDiagnostics(), ideProto.getIndex(), + ideProto.getVersion(), ideProto.getIsLocalFetch(), ideProto.getIsDiskErrorAtSource()); break; case TASK_ATTEMPT_FAILED_EVENT: TaskAttemptFailedEventProto tfProto = diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/Fetcher.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/Fetcher.java index f2412e35d9..7f2033f976 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/Fetcher.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/Fetcher.java @@ -66,7 +66,7 @@ import org.apache.tez.runtime.library.common.sort.impl.TezSpillRecord; import org.apache.tez.runtime.library.exceptions.FetcherReadTimeoutException; import org.apache.tez.runtime.library.common.shuffle.FetchedInput.Type; - +import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import org.apache.tez.common.Preconditions; /** @@ -277,7 +277,8 @@ public FetchResult callInternal() throws Exception { HostFetchResult hostFetchResult; - if (localDiskFetchEnabled && host.equals(localHostname) && port == shufflePort) { + boolean isLocalFetch = localDiskFetchEnabled && host.equals(localHostname) && port == shufflePort; + if (isLocalFetch) { hostFetchResult = setupLocalDiskFetch(); } else if (multiplex) { hostFetchResult = doSharedFetch(); @@ -288,7 +289,7 @@ public FetchResult callInternal() throws Exception { if (hostFetchResult.failedInputs != null && hostFetchResult.failedInputs.length > 0) { if (!isShutDown.get()) { LOG.warn("copyInputs failed for tasks " + Arrays.toString(hostFetchResult.failedInputs)); - for (InputAttemptIdentifier left : hostFetchResult.failedInputs) { + for (InputAttemptFetchFailure left : hostFetchResult.failedInputs) { fetcherCallback.fetchFailed(host, left, hostFetchResult.connectFailed); } } else { @@ -504,7 +505,7 @@ private HostFetchResult setupConnection(Collection attem // ioErrs.increment(1); // If connect did not succeed, just mark all the maps as failed, // indirectly penalizing the host - InputAttemptIdentifier[] failedFetches = null; + InputAttemptFetchFailure[] failedFetches = null; if (isShutDown.get()) { if (isDebugEnabled) { LOG.debug( @@ -512,8 +513,7 @@ private HostFetchResult setupConnection(Collection attem e.getClass().getName() + ", Message: " + e.getMessage()); } } else { - failedFetches = srcAttemptsRemaining.values(). - toArray(new InputAttemptIdentifier[srcAttemptsRemaining.values().size()]); + failedFetches = InputAttemptFetchFailure.fromAttempts(srcAttemptsRemaining.values()); } return new HostFetchResult(new FetchResult(host, port, partition, partitionCount, srcAttemptsRemaining.values()), failedFetches, true); } @@ -546,7 +546,7 @@ private HostFetchResult setupConnection(Collection attem LOG.warn("Fetch Failure from host while connecting: " + host + ", attempt: " + firstAttempt + " Informing ShuffleManager: ", e); return new HostFetchResult(new FetchResult(host, port, partition, partitionCount, srcAttemptsRemaining.values()), - new InputAttemptIdentifier[] { firstAttempt }, false); + new InputAttemptFetchFailure[] { new InputAttemptFetchFailure(firstAttempt) }, false); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); //reset status @@ -583,7 +583,7 @@ protected HostFetchResult doHttpFetch(CachingCallBack callback) { // On any error, faildTasks is not null and we exit // after putting back the remaining maps to the // yet_to_be_fetched list and marking the failed tasks. - InputAttemptIdentifier[] failedInputs = null; + InputAttemptFetchFailure[] failedInputs = null; while (!srcAttemptsRemaining.isEmpty() && failedInputs == null) { InputAttemptIdentifier inputAttemptIdentifier = srcAttemptsRemaining.entrySet().iterator().next().getValue(); @@ -710,7 +710,7 @@ public void freeResources(FetchedInput fetchedInput) { } } - InputAttemptIdentifier[] failedFetches = null; + InputAttemptFetchFailure[] failedFetches = null; if (failMissing && srcAttemptsRemaining.size() > 0) { if (isShutDown.get()) { if (isDebugEnabled) { @@ -719,8 +719,8 @@ public void freeResources(FetchedInput fetchedInput) { " remaining inputs"); } } else { - failedFetches = srcAttemptsRemaining.values(). - toArray(new InputAttemptIdentifier[srcAttemptsRemaining.values().size()]); + failedFetches = + InputAttemptFetchFailure.fromAttemptsLocalFetchFailure(srcAttemptsRemaining.values()); } } else { // nothing needs to be done to requeue remaining entries @@ -769,10 +769,10 @@ public Map getPathToAttemptMap() { static class HostFetchResult { private final FetchResult fetchResult; - private final InputAttemptIdentifier[] failedInputs; + private final InputAttemptFetchFailure[] failedInputs; private final boolean connectFailed; - public HostFetchResult(FetchResult fetchResult, InputAttemptIdentifier[] failedInputs, + public HostFetchResult(FetchResult fetchResult, InputAttemptFetchFailure[] failedInputs, boolean connectFailed) { this.fetchResult = fetchResult; this.failedInputs = failedInputs; @@ -830,8 +830,11 @@ public String toString() { return "id: " + srcAttemptId + ", decompressed length: " + decompressedLength + ", compressed length: " + compressedLength + ", reduce: " + forReduce; } } - private InputAttemptIdentifier[] fetchInputs(DataInputStream input, - CachingCallBack callback, InputAttemptIdentifier inputAttemptIdentifier) throws FetcherReadTimeoutException { + + @VisibleForTesting + InputAttemptFetchFailure[] fetchInputs(DataInputStream input, CachingCallBack callback, + InputAttemptIdentifier inputAttemptIdentifier) + throws FetcherReadTimeoutException { FetchedInput fetchedInput = null; InputAttemptIdentifier srcAttemptId = null; long decompressedLength = 0; @@ -855,9 +858,19 @@ private InputAttemptIdentifier[] fetchInputs(DataInputStream input, header.readFields(input); pathComponent = header.getMapId(); if (!pathComponent.startsWith(InputAttemptIdentifier.PATH_PREFIX)) { - throw new IllegalArgumentException("Invalid map id: " + header.getMapId() + ", expected to start with " + - InputAttemptIdentifier.PATH_PREFIX + ", partition: " + header.getPartition() - + " while fetching " + inputAttemptIdentifier); + if (pathComponent.startsWith(ShuffleHandlerError.DISK_ERROR_EXCEPTION.toString())) { + LOG.warn("Invalid map id: " + header.getMapId() + ", expected to start with " + + InputAttemptIdentifier.PATH_PREFIX + ", partition: " + header.getPartition() + + " while fetching " + inputAttemptIdentifier); + // this should be treated as local fetch failure while reporting later + return new InputAttemptFetchFailure[] { + InputAttemptFetchFailure.fromDiskErrorAtSource(inputAttemptIdentifier) }; + } else { + throw new IllegalArgumentException( + "Invalid map id: " + header.getMapId() + ", expected to start with " + + InputAttemptIdentifier.PATH_PREFIX + ", partition: " + header.getPartition() + + " while fetching " + inputAttemptIdentifier); + } } srcAttemptId = pathToAttemptMap.get(new PathPartition(pathComponent, header.getPartition())); @@ -882,7 +895,7 @@ private InputAttemptIdentifier[] fetchInputs(DataInputStream input, if (!isShutDown.get()) { LOG.warn("Invalid src id ", e); // Don't know which one was bad, so consider all of them as bad - return srcAttemptsRemaining.values().toArray(new InputAttemptIdentifier[srcAttemptsRemaining.size()]); + return InputAttemptFetchFailure.fromAttempts(srcAttemptsRemaining.values()); } else { if (isDebugEnabled) { LOG.debug("Already shutdown. Ignoring badId error with message: " + e.getMessage()); @@ -901,7 +914,8 @@ private InputAttemptIdentifier[] fetchInputs(DataInputStream input, srcAttemptId = getNextRemainingAttempt(); } assert (srcAttemptId != null); - return new InputAttemptIdentifier[]{srcAttemptId}; + return new InputAttemptFetchFailure[] { + InputAttemptFetchFailure.fromAttempt(srcAttemptId) }; } else { if (isDebugEnabled) { LOG.debug("Already shutdown. Ignoring verification failure."); @@ -1003,10 +1017,10 @@ private InputAttemptIdentifier[] fetchInputs(DataInputStream input, // Cleanup the fetchedInput before returning. cleanupFetchedInput(fetchedInput); if (srcAttemptId == null) { - return srcAttemptsRemaining.values() - .toArray(new InputAttemptIdentifier[srcAttemptsRemaining.size()]); + return InputAttemptFetchFailure.fromAttempts(srcAttemptsRemaining.values()); } else { - return new InputAttemptIdentifier[] { srcAttemptId }; + return new InputAttemptFetchFailure[] { + new InputAttemptFetchFailure(srcAttemptId) }; } } LOG.warn("Failed to shuffle output of " + srcAttemptId + " from " + host, @@ -1015,7 +1029,8 @@ private InputAttemptIdentifier[] fetchInputs(DataInputStream input, // Cleanup the fetchedInput cleanupFetchedInput(fetchedInput); // metrics.failedFetch(); - return new InputAttemptIdentifier[] { srcAttemptId }; + return new InputAttemptFetchFailure[] { + new InputAttemptFetchFailure(srcAttemptId) }; } return null; } diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/FetcherCallback.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/FetcherCallback.java index 34bd272909..b751fb9ce0 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/FetcherCallback.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/FetcherCallback.java @@ -28,6 +28,7 @@ public void fetchSucceeded(String host, InputAttemptIdentifier srcAttemptIdentif FetchedInput fetchedInput, long fetchedBytes, long decompressedLength, long copyDuration) throws IOException; - public void fetchFailed(String host, InputAttemptIdentifier srcAttemptIdentifier, boolean connectFailed); + public void fetchFailed(String host, InputAttemptFetchFailure srcAttemptFetchFailure, + boolean connectFailed); } diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java new file mode 100644 index 0000000000..d94db35c2f --- /dev/null +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java @@ -0,0 +1,115 @@ +/** + * 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.tez.runtime.library.common.shuffle; + +import java.util.Arrays; +import java.util.Collection; + +import org.apache.tez.runtime.library.common.CompositeInputAttemptIdentifier; +import org.apache.tez.runtime.library.common.InputAttemptIdentifier; + +/** + * InputAttemptFetchFailure is supposed to wrap an InputAttemptIdentifier with any kind of failure + * information during fetch. It can be useful for propagating as a single object instead of multiple + * parameters (local fetch error, remote fetch error, connect failed, read failed, etc.). + */ +public class InputAttemptFetchFailure { + + private final InputAttemptIdentifier inputAttemptIdentifier; + private final boolean isLocalFetch; + private final boolean isDiskErrorAtSource; + + public InputAttemptFetchFailure(InputAttemptIdentifier inputAttemptIdentifier) { + this(inputAttemptIdentifier, false, false); + } + + public InputAttemptFetchFailure(InputAttemptIdentifier inputAttemptIdentifier, + boolean isLocalFetch, boolean isDiskErrorAtSource) { + this.inputAttemptIdentifier = inputAttemptIdentifier; + this.isLocalFetch = isLocalFetch; + this.isDiskErrorAtSource = isDiskErrorAtSource; + } + + public InputAttemptIdentifier getInputAttemptIdentifier() { + return inputAttemptIdentifier; + } + + public boolean isLocalFetch() { + return isLocalFetch; + } + + public boolean isDiskErrorAtSource() { + return isDiskErrorAtSource; + } + + public static InputAttemptFetchFailure fromAttempt(InputAttemptIdentifier attempt) { + return new InputAttemptFetchFailure(attempt, false, false); + } + + public static InputAttemptFetchFailure fromLocalFetchFailure(InputAttemptIdentifier attempt) { + return new InputAttemptFetchFailure(attempt, true, false); + } + + public static InputAttemptFetchFailure fromDiskErrorAtSource(InputAttemptIdentifier attempt) { + return new InputAttemptFetchFailure(attempt, false, true); + } + + public static InputAttemptFetchFailure[] fromAttempts(Collection values) { + return values.stream().map(identifier -> new InputAttemptFetchFailure(identifier, false, false)) + .toArray(InputAttemptFetchFailure[]::new); + } + + public static InputAttemptFetchFailure[] fromAttempts(InputAttemptIdentifier[] values) { + return Arrays.asList(values).stream() + .map(identifier -> new InputAttemptFetchFailure(identifier, false, false)) + .toArray(InputAttemptFetchFailure[]::new); + } + + public static InputAttemptFetchFailure[] fromAttemptsLocalFetchFailure( + Collection values) { + return values.stream().map(identifier -> new InputAttemptFetchFailure(identifier, true, false)) + .toArray(InputAttemptFetchFailure[]::new); + } + + public static InputAttemptFetchFailure fromCompositeAttemptLocalFetchFailure( + CompositeInputAttemptIdentifier compositeInputAttemptIdentifier) { + return new InputAttemptFetchFailure(compositeInputAttemptIdentifier, true, false); + } + + @Override + public boolean equals(Object obj) { + if (obj == null || (obj.getClass() != this.getClass())) { + return false; + } + return inputAttemptIdentifier.equals(((InputAttemptFetchFailure) obj).inputAttemptIdentifier) + && isLocalFetch == ((InputAttemptFetchFailure) obj).isLocalFetch + && isDiskErrorAtSource == ((InputAttemptFetchFailure) obj).isDiskErrorAtSource; + } + + @Override + public int hashCode() { + return 31 * inputAttemptIdentifier.hashCode() + 31 * (isLocalFetch ? 0 : 1) + + 31 * (isDiskErrorAtSource ? 0 : 1); + } + + @Override + public String toString() { + return String.format("%s, isLocalFetch: %s, isDiskErrorAtSource: %s", + inputAttemptIdentifier.toString(), isLocalFetch, isDiskErrorAtSource); + } +} diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java index 9b65103888..ff64b7f664 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java @@ -80,6 +80,7 @@ import org.apache.tez.runtime.library.common.shuffle.Fetcher.FetcherBuilder; import org.apache.tez.runtime.library.common.shuffle.FetcherCallback; import org.apache.tez.runtime.library.common.shuffle.HostPort; +import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.InputHost; import org.apache.tez.runtime.library.common.shuffle.InputHost.PartitionToInputs; import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; @@ -791,12 +792,15 @@ private void reportFatalError(Throwable exception, String message) { @Override public void fetchFailed(String host, - InputAttemptIdentifier srcAttemptIdentifier, boolean connectFailed) { + InputAttemptFetchFailure inputAttemptFetchFailure, boolean connectFailed) { // TODO NEWTEZ. Implement logic to report fetch failures after a threshold. // For now, reporting immediately. - LOG.info(srcNameTrimmed + ": " + "Fetch failed for src: " + srcAttemptIdentifier - + "InputIdentifier: " + srcAttemptIdentifier + ", connectFailed: " - + connectFailed); + InputAttemptIdentifier srcAttemptIdentifier = inputAttemptFetchFailure.getInputAttemptIdentifier(); + LOG.info( + "{}: Fetch failed for src: {} InputIdentifier: {}, connectFailed: {}, " + + "local fetch: {}, remote fetch failure reported as local failure: {})", + srcNameTrimmed, srcAttemptIdentifier, srcAttemptIdentifier, connectFailed, + inputAttemptFetchFailure.isLocalFetch(), inputAttemptFetchFailure.isDiskErrorAtSource()); failedShufflesCounter.increment(1); inputContext.notifyProgress(); if (srcAttemptIdentifier == null) { @@ -809,7 +813,9 @@ public void fetchFailed(String host, srcAttemptIdentifier.getInputIdentifier(), srcAttemptIdentifier.getAttemptNumber()), srcAttemptIdentifier.getInputIdentifier(), - srcAttemptIdentifier.getAttemptNumber()); + srcAttemptIdentifier.getAttemptNumber(), + inputAttemptFetchFailure.isLocalFetch(), + inputAttemptFetchFailure.isDiskErrorAtSource()); List failedEvents = Lists.newArrayListWithCapacity(1); failedEvents.add(readError); diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java index 1fcd621900..aaa3a3b217 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java @@ -50,7 +50,9 @@ import org.apache.tez.runtime.library.common.sort.impl.TezIndexRecord; import org.apache.tez.runtime.library.common.sort.impl.TezSpillRecord; import org.apache.tez.runtime.library.exceptions.FetcherReadTimeoutException; +import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; +import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import com.google.common.annotations.VisibleForTesting; @@ -272,7 +274,8 @@ protected void copyFromHost(MapHost host) throws IOException { // On any error, faildTasks is not null and we exit // after putting back the remaining maps to the // yet_to_be_fetched list and marking the failed tasks. - InputAttemptIdentifier[] failedTasks = null; + InputAttemptFetchFailure[] failedTasks = null; + while (!remaining.isEmpty() && failedTasks == null) { InputAttemptIdentifier inputAttemptIdentifier = remaining.entrySet().iterator().next().getValue(); @@ -295,25 +298,14 @@ protected void copyFromHost(MapHost host) throws IOException { LOG.debug("Not reporting connection re-establishment failure since fetcher is stopped"); return; } - failedTasks = new InputAttemptIdentifier[] {getNextRemainingAttempt()}; + failedTasks = new InputAttemptFetchFailure[] { + new InputAttemptFetchFailure(getNextRemainingAttempt()) }; break; } } } - if (failedTasks != null && failedTasks.length > 0) { - if (stopped) { - if (LOG.isDebugEnabled()) { - LOG.debug("Ignoring copyMapOutput failures for tasks: " + Arrays.toString(failedTasks) + - " since Fetcher has been stopped"); - } - } else { - LOG.warn("copyMapOutput failed for tasks " + Arrays.toString(failedTasks)); - for (InputAttemptIdentifier left : failedTasks) { - scheduler.copyFailed(left, host, true, false, false); - } - } - } + invokeCopyFailedForFailedTasks(host, failedTasks); cleanupCurrentConnection(false); @@ -327,6 +319,23 @@ protected void copyFromHost(MapHost host) throws IOException { } } + private void invokeCopyFailedForFailedTasks(MapHost host, + InputAttemptFetchFailure[] failedTasks) { + if (failedTasks != null && failedTasks.length > 0) { + if (stopped) { + if (LOG.isDebugEnabled()) { + LOG.debug("Ignoring copyMapOutput failures for tasks: " + Arrays.toString(failedTasks) + + " since Fetcher has been stopped"); + } + } else { + LOG.warn("copyMapOutput failed for tasks " + Arrays.toString(failedTasks)); + for (InputAttemptFetchFailure left : failedTasks) { + scheduler.copyFailed(left, host, true, false); + } + } + } + } + @VisibleForTesting boolean setupConnection(MapHost host, Collection attempts) throws IOException { @@ -369,7 +378,8 @@ boolean setupConnection(MapHost host, Collection attempt for (InputAttemptIdentifier left : remaining.values()) { // Need to be handling temporary glitches .. // Report read error to the AM to trigger source failure heuristics - scheduler.copyFailed(left, host, connectSucceeded, !connectSucceeded, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(left), host, connectSucceeded, + !connectSucceeded); } return false; } @@ -393,7 +403,8 @@ protected void putBackRemainingMapOutputs(MapHost host) { } } - private static InputAttemptIdentifier[] EMPTY_ATTEMPT_ID_ARRAY = new InputAttemptIdentifier[0]; + private static final InputAttemptFetchFailure[] EMPTY_ATTEMPT_ID_ARRAY = + new InputAttemptFetchFailure[0]; private static class MapOutputStat { final InputAttemptIdentifier srcAttemptId; @@ -414,8 +425,8 @@ public String toString() { } } - protected InputAttemptIdentifier[] copyMapOutput(MapHost host, - DataInputStream input, InputAttemptIdentifier inputAttemptIdentifier) throws FetcherReadTimeoutException { + protected InputAttemptFetchFailure[] copyMapOutput(MapHost host, DataInputStream input, + InputAttemptIdentifier inputAttemptIdentifier) throws FetcherReadTimeoutException { MapOutput mapOutput = null; InputAttemptIdentifier srcAttemptId = null; long decompressedLength = 0; @@ -441,7 +452,13 @@ protected InputAttemptIdentifier[] copyMapOutput(MapHost host, badIdErrs.increment(1); LOG.warn("Invalid map id: " + header.mapId + ", expected to start with " + InputAttemptIdentifier.PATH_PREFIX + ", partition: " + header.forReduce); - return new InputAttemptIdentifier[]{getNextRemainingAttempt()}; + if (header.mapId.startsWith(ShuffleHandlerError.DISK_ERROR_EXCEPTION.toString())) { + //this should be treated as local fetch failure while reporting later + return new InputAttemptFetchFailure[] { + InputAttemptFetchFailure.fromDiskErrorAtSource(getNextRemainingAttempt()) }; + } + return new InputAttemptFetchFailure[] { + InputAttemptFetchFailure.fromAttempt(getNextRemainingAttempt()) }; } else { LOG.debug("Already shutdown. Ignoring invalid map id error"); return EMPTY_ATTEMPT_ID_ARRAY; @@ -464,7 +481,8 @@ protected InputAttemptIdentifier[] copyMapOutput(MapHost host, LOG.warn("Invalid map id ", e); // Don't know which one was bad, so consider this one bad and dont read // the remaining because we dont know where to start reading from. YARN-1773 - return new InputAttemptIdentifier[]{getNextRemainingAttempt()}; + return new InputAttemptFetchFailure[] { + new InputAttemptFetchFailure(getNextRemainingAttempt()) }; } else { if (LOG.isDebugEnabled()) { LOG.debug("Already shutdown. Ignoring invalid map id error. Exception: " + @@ -484,7 +502,8 @@ protected InputAttemptIdentifier[] copyMapOutput(MapHost host, LOG.warn("Was expecting " + srcAttemptId + " but got null"); } assert (srcAttemptId != null); - return new InputAttemptIdentifier[]{srcAttemptId}; + return new InputAttemptFetchFailure[] { + new InputAttemptFetchFailure(getNextRemainingAttempt()) }; } else { LOG.debug("Already stopped. Ignoring verification failure."); return EMPTY_ATTEMPT_ID_ARRAY; @@ -578,9 +597,10 @@ protected InputAttemptIdentifier[] copyMapOutput(MapHost host, srcAttemptId + " decomp: " + decompressedLength + ", " + compressedLength, ioe); if (srcAttemptId == null) { - return remaining.values().toArray(new InputAttemptIdentifier[remaining.values().size()]); + return InputAttemptFetchFailure.fromAttempts(remaining.values()); } else { - return new InputAttemptIdentifier[]{srcAttemptId}; + return new InputAttemptFetchFailure[] { + new InputAttemptFetchFailure(srcAttemptId) }; } } LOG.warn("Failed to shuffle output of " + srcAttemptId + @@ -588,7 +608,8 @@ protected InputAttemptIdentifier[] copyMapOutput(MapHost host, // Inform the shuffle-scheduler mapOutput.abort(); - return new InputAttemptIdentifier[] {srcAttemptId}; + return new InputAttemptFetchFailure[] { + new InputAttemptFetchFailure(srcAttemptId) }; } return null; } @@ -717,7 +738,8 @@ protected void setupLocalDiskFetch(MapHost host) throws InterruptedException { if (!stopped) { hasFailures = true; ioErrs.increment(1); - scheduler.copyFailed(srcAttemptId, host, true, false, true); + scheduler.copyFailed(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttemptId), + host, true, false); LOG.warn("Failed to read local disk output of " + srcAttemptId + " from " + host.getHostIdentifier(), e); } else { diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleHeader.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleHeader.java index 9f883dbfdc..f074e897e8 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleHeader.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleHeader.java @@ -103,4 +103,11 @@ public void write(DataOutput out) throws IOException { WritableUtils.writeVLong(out, uncompressedLength); WritableUtils.writeVInt(out, forReduce); } + + @Override + public String toString() { + return String.format( + "ShuffleHeader [mapId=%s, uncompressedLength=%d, compressedLength=%d, forReduce=%d]", mapId, + uncompressedLength, compressedLength, forReduce); + } } diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java index 736f41b6eb..a16d0db162 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java @@ -80,6 +80,7 @@ import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils.FetchStatsLogger; import org.apache.tez.runtime.library.common.shuffle.HostPort; +import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.MapHost.HostPortPartition; import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.MapOutput.Type; @@ -755,16 +756,13 @@ private void logProgress() { } } - public synchronized void copyFailed(InputAttemptIdentifier srcAttempt, - MapHost host, - boolean readError, - boolean connectError, - boolean isLocalFetch) { + public synchronized void copyFailed(InputAttemptFetchFailure fetchFailure, MapHost host, + boolean readError, boolean connectError) { failedShuffleCounter.increment(1); inputContext.notifyProgress(); - int failures = incrementAndGetFailureAttempt(srcAttempt); + int failures = incrementAndGetFailureAttempt(fetchFailure.getInputAttemptIdentifier()); - if (!isLocalFetch) { + if (!fetchFailure.isLocalFetch()) { /** * Track the number of failures that has happened since last completion. * This gets reset on a successful copy. @@ -789,11 +787,11 @@ public synchronized void copyFailed(InputAttemptIdentifier srcAttempt, if (shouldInformAM) { //Inform AM. In case producer needs to be restarted, it is handled at AM. - informAM(srcAttempt); + informAM(fetchFailure); } //Restart consumer in case shuffle is not healthy - if (!isShuffleHealthy(srcAttempt)) { + if (!isShuffleHealthy(fetchFailure.getInputAttemptIdentifier())) { return; } @@ -868,21 +866,24 @@ public void reportLocalError(IOException ioe) { } // Notify AM - private void informAM(InputAttemptIdentifier srcAttempt) { + private void informAM(InputAttemptFetchFailure fetchFailure) { + InputAttemptIdentifier srcAttempt = fetchFailure.getInputAttemptIdentifier(); LOG.info( - srcNameTrimmed + ": " + "Reporting fetch failure for InputIdentifier: " - + srcAttempt + " taskAttemptIdentifier: " + TezRuntimeUtils - .getTaskAttemptIdentifier(inputContext.getSourceVertexName(), - srcAttempt.getInputIdentifier(), - srcAttempt.getAttemptNumber()) + " to AM."); + "{}: Reporting fetch failure for InputIdentifier: {} taskAttemptIdentifier: {}, " + + "local fetch: {}, remote fetch failure reported as local failure: {}) to AM.", + srcNameTrimmed, srcAttempt, + TezRuntimeUtils.getTaskAttemptIdentifier(inputContext.getSourceVertexName(), + srcAttempt.getInputIdentifier(), srcAttempt.getAttemptNumber()), + fetchFailure.isLocalFetch(), fetchFailure.isDiskErrorAtSource()); List failedEvents = Lists.newArrayListWithCapacity(1); - failedEvents.add(InputReadErrorEvent.create( - "Fetch failure for " + TezRuntimeUtils - .getTaskAttemptIdentifier(inputContext.getSourceVertexName(), - srcAttempt.getInputIdentifier(), - srcAttempt.getAttemptNumber()) + " to jobtracker.", - srcAttempt.getInputIdentifier(), - srcAttempt.getAttemptNumber())); + failedEvents.add( + InputReadErrorEvent.create( + "Fetch failure for " + + TezRuntimeUtils.getTaskAttemptIdentifier(inputContext.getSourceVertexName(), + srcAttempt.getInputIdentifier(), srcAttempt.getAttemptNumber()) + + " to jobtracker.", + srcAttempt.getInputIdentifier(), srcAttempt.getAttemptNumber(), + fetchFailure.isLocalFetch(), fetchFailure.isDiskErrorAtSource())); inputContext.sendEvents(failedEvents); } diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestFetcher.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestFetcher.java index db9c7afad0..05d4eb4145 100644 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestFetcher.java +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestFetcher.java @@ -32,6 +32,7 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import java.io.DataInputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -46,10 +47,16 @@ import org.apache.hadoop.fs.LocalFileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.yarn.api.records.ApplicationId; +import org.apache.tez.common.counters.TezCounters; import org.apache.tez.dag.api.TezConfiguration; +import org.apache.tez.runtime.api.InputContext; import org.apache.tez.runtime.library.api.TezRuntimeConfiguration; import org.apache.tez.runtime.library.common.InputAttemptIdentifier; +import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; +import org.apache.tez.runtime.library.common.shuffle.impl.ShuffleManager; +import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.ShuffleHeader; import org.apache.tez.runtime.library.common.sort.impl.TezIndexRecord; +import org.apache.tez.runtime.library.testutils.RuntimeTestUtils; import org.junit.Assert; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -79,7 +86,8 @@ public void testLocalFetchModeSetting() throws Exception { Fetcher fetcher = spy(builder.build()); FetchResult fr = new FetchResult(HOST, PORT, 0, 1, Arrays.asList(srcAttempts)); - Fetcher.HostFetchResult hfr = new Fetcher.HostFetchResult(fr, srcAttempts, false); + Fetcher.HostFetchResult hfr = + new Fetcher.HostFetchResult(fr, InputAttemptFetchFailure.fromAttempts(srcAttempts), false); doReturn(hfr).when(fetcher).setupLocalDiskFetch(); doReturn(null).when(fetcher).doHttpFetch(); doNothing().when(fetcher).shutdown(); @@ -151,7 +159,7 @@ public void testSetupLocalDiskFetch() throws Exception { }; final int FIRST_FAILED_ATTEMPT_IDX = 2; final int SECOND_FAILED_ATTEMPT_IDX = 4; - final int[] sucessfulAttempts = {0, 1, 3}; + final int[] successfulAttempts = {0, 1, 3}; TezConfiguration conf = new TezConfiguration(); conf.set(TezRuntimeConfiguration.TEZ_RUNTIME_OPTIMIZE_LOCAL_FETCH, "true"); @@ -206,18 +214,24 @@ public TezIndexRecord answer(InvocationOnMock invocation) throws Throwable { doNothing().when(fetcher).shutdown(); doNothing().when(callback).fetchSucceeded(anyString(), any(InputAttemptIdentifier.class), any(FetchedInput.class), anyLong(), anyLong(), anyLong()); - doNothing().when(callback).fetchFailed(anyString(), any(InputAttemptIdentifier.class), eq(false)); + doNothing().when(callback).fetchFailed(anyString(), any(InputAttemptFetchFailure.class), eq(false)); FetchResult fetchResult = fetcher.call(); verify(fetcher).setupLocalDiskFetch(); - // expect 3 sucesses and 2 failures - for (int i : sucessfulAttempts) { + // expect 3 successes and 2 failures + for (int i : successfulAttempts) { verifyFetchSucceeded(callback, srcAttempts[i], conf); } - verify(callback).fetchFailed(eq(HOST), eq(srcAttempts[FIRST_FAILED_ATTEMPT_IDX]), eq(false)); - verify(callback).fetchFailed(eq(HOST), eq(srcAttempts[SECOND_FAILED_ATTEMPT_IDX]), eq(false)); + verify(callback).fetchFailed(eq(HOST), + eq(InputAttemptFetchFailure + .fromCompositeAttemptLocalFetchFailure(srcAttempts[FIRST_FAILED_ATTEMPT_IDX])), + eq(false)); + verify(callback).fetchFailed(eq(HOST), + eq(InputAttemptFetchFailure + .fromCompositeAttemptLocalFetchFailure(srcAttempts[SECOND_FAILED_ATTEMPT_IDX])), + eq(false)); Assert.assertEquals("fetchResult host", fetchResult.getHost(), HOST); Assert.assertEquals("fetchResult partition", fetchResult.getPartition(), partition); @@ -304,4 +318,30 @@ public void testInputAttemptIdentifierMap() { Assert.assertTrue(expectedSrcAttempts[count++].toString().compareTo(key) == 0); } } + + @Test + public void testShuffleHandlerDiskErrorUnordered() + throws Exception { + Configuration conf = new Configuration(); + + InputContext inputContext = mock(InputContext.class); + doReturn(new TezCounters()).when(inputContext).getCounters(); + doReturn("vertex").when(inputContext).getSourceVertexName(); + + Fetcher.FetcherBuilder builder = new Fetcher.FetcherBuilder(mock(ShuffleManager.class), null, + null, ApplicationId.newInstance(0, 1), 1, null, "fetcherTest", conf, true, HOST, PORT, + false, true, false); + builder.assignWork(HOST, PORT, 0, 1, Arrays.asList(new InputAttemptIdentifier(0, 0))); + + Fetcher fetcher = builder.build(); + ShuffleHeader header = + new ShuffleHeader(ShuffleHandlerError.DISK_ERROR_EXCEPTION.toString(), -1, -1, -1); + DataInputStream input = RuntimeTestUtils.shuffleHeaderToDataInput(header); + + InputAttemptFetchFailure[] failures = + fetcher.fetchInputs(input, null, new InputAttemptIdentifier(0, 0)); + Assert.assertEquals(1, failures.length); + Assert.assertTrue(failures[0].isDiskErrorAtSource()); + Assert.assertFalse(failures[0].isLocalFetch()); + } } diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java index 103f83d3cc..4d84256a47 100644 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java @@ -52,7 +52,6 @@ import org.apache.tez.common.security.JobTokenIdentifier; import org.apache.tez.common.security.JobTokenSecretManager; import org.apache.tez.dag.api.TezConfiguration; -import org.apache.tez.dag.api.TezConstants; import org.apache.tez.runtime.api.Event; import org.apache.tez.runtime.api.ExecutionContext; import org.apache.tez.runtime.api.InputContext; @@ -62,9 +61,9 @@ import org.apache.tez.runtime.library.common.shuffle.FetchedInput; import org.apache.tez.runtime.library.common.shuffle.FetchedInputAllocator; import org.apache.tez.runtime.library.common.shuffle.Fetcher; +import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.FetchResult; import org.apache.tez.runtime.library.common.shuffle.InputHost; -import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; import org.apache.tez.runtime.library.shuffle.impl.ShuffleUserPayloads.DataMovementEventPayloadProto; import org.junit.After; import org.junit.Before; diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestFetcher.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestFetcher.java index 5f7fe4ba7e..028fbce96a 100644 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestFetcher.java +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestFetcher.java @@ -42,6 +42,7 @@ import java.net.URL; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedList; @@ -72,7 +73,10 @@ import org.apache.tez.runtime.library.common.security.SecureShuffleUtils; import org.apache.tez.runtime.library.common.sort.impl.TezIndexRecord; import org.apache.tez.runtime.library.exceptions.FetcherReadTimeoutException; +import org.apache.tez.runtime.library.testutils.RuntimeTestUtils; +import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; +import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import org.junit.Assert; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -241,7 +245,7 @@ public void testSetupLocalDiskFetch() throws Exception { ); final int FIRST_FAILED_ATTEMPT_IDX = 2; final int SECOND_FAILED_ATTEMPT_IDX = 4; - final int[] sucessfulAttemptsIndexes = { 0, 1, 3 }; + final int[] successfulAttemptsIndexes = { 0, 1, 3 }; doReturn(srcAttempts).when(scheduler).getMapsForHost(host); @@ -311,13 +315,17 @@ public TezIndexRecord answer(InvocationOnMock invocation) throws Throwable { spyFetcher.setupLocalDiskFetch(host); // should have exactly 3 success and 1 failure. - for (int i : sucessfulAttemptsIndexes) { + for (int i : successfulAttemptsIndexes) { for (int j = 0; j < host.getPartitionCount(); j++) { verifyCopySucceeded(scheduler, host, srcAttempts, i, j); } } - verify(scheduler).copyFailed(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(0), host, true, false, true); - verify(scheduler).copyFailed(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(0), host, true, false, true); + verify(scheduler).copyFailed( + eq(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(0))), + eq(host), eq(true), eq(false)); + verify(scheduler).copyFailed( + eq(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(0))), + eq(host), eq(true), eq(false)); verify(spyFetcher).putBackRemainingMapOutputs(host); verify(scheduler).putBackKnownMapOutput(host, srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX)); @@ -426,7 +434,7 @@ public void testSetupLocalDiskFetchAutoReduce() throws Exception { ); final int FIRST_FAILED_ATTEMPT_IDX = 2; final int SECOND_FAILED_ATTEMPT_IDX = 4; - final int[] sucessfulAttemptsIndexes = { 0, 1, 3 }; + final int[] successfulAttemptsIndexes = { 0, 1, 3 }; doReturn(srcAttempts).when(scheduler).getMapsForHost(host); final ConcurrentMap pathToIdentifierMap @@ -503,15 +511,23 @@ public TezIndexRecord answer(InvocationOnMock invocation) throws Throwable { spyFetcher.setupLocalDiskFetch(host); // should have exactly 3 success and 1 failure. - for (int i : sucessfulAttemptsIndexes) { + for (int i : successfulAttemptsIndexes) { for (int j = 0; j < host.getPartitionCount(); j++) { verifyCopySucceeded(scheduler, host, srcAttempts, i, j); } } - verify(scheduler).copyFailed(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(0), host, true, false, true); - verify(scheduler).copyFailed(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(1), host, true, false, true); - verify(scheduler).copyFailed(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(0), host, true, false, true); - verify(scheduler).copyFailed(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(1), host, true, false, true); + verify(scheduler).copyFailed( + eq(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(0))), + eq(host), eq(true), eq(false)); + verify(scheduler).copyFailed( + eq(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(1))), + eq(host), eq(true), eq(false)); + verify(scheduler).copyFailed(eq( + InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(0))), + eq(host), eq(true), eq(false)); + verify(scheduler).copyFailed(eq( + InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(1))), + eq(host), eq(true), eq(false)); verify(spyFetcher).putBackRemainingMapOutputs(host); verify(scheduler).putBackKnownMapOutput(host, srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX)); @@ -630,8 +646,8 @@ public MapOutput answer(InvocationOnMock invocation) throws Throwable { //setup connection should be called twice (1 for connect and another for retry) verify(fetcher, times(2)).setupConnection(any(MapHost.class), any(Collection.class)); //since copyMapOutput consistently fails, it should call copyFailed once - verify(scheduler, times(1)).copyFailed(any(InputAttemptIdentifier.class), any(MapHost.class), - anyBoolean(), anyBoolean(), anyBoolean()); + verify(scheduler, times(1)).copyFailed(any(InputAttemptFetchFailure.class), any(MapHost.class), + anyBoolean(), anyBoolean()); verify(fetcher, times(1)).putBackRemainingMapOutputs(any(MapHost.class)); verify(scheduler, times(3)).putBackKnownMapOutput(any(MapHost.class), @@ -750,6 +766,32 @@ public void testInputAttemptIdentifierMap() { } } + @Test + public void testShuffleHandlerDiskErrorOrdered() + throws Exception { + MapHost mapHost = new MapHost(HOST, PORT, 0, 1); + InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(0, 0, "attempt"); + + FetcherOrderedGrouped fetcher = new FetcherOrderedGrouped(null, null, null, null, null, false, + 0, null, new TezConfiguration(), null, false, HOST, PORT, "src vertex", mapHost, + ioErrsCounter, wrongLengthErrsCounter, badIdErrsCounter, wrongMapErrsCounter, + connectionErrsCounter, wrongReduceErrsCounter, APP_ID, DAG_ID, false, false, true, false); + fetcher.remaining = new HashMap(); + + ShuffleHeader header = + new ShuffleHeader(ShuffleHandlerError.DISK_ERROR_EXCEPTION.toString(), -1, -1, -1); + DataInputStream input = RuntimeTestUtils.shuffleHeaderToDataInput(header); + + // copyMapOutput is used for remote fetch, this time it returns a fetch failure, which is fatal + // and should be treated as a local fetch failure + InputAttemptFetchFailure[] failures = + fetcher.copyMapOutput(mapHost, input, inputAttemptIdentifier); + + Assert.assertEquals(1, failures.length); + Assert.assertTrue(failures[0].isDiskErrorAtSource()); + Assert.assertFalse(failures[0].isLocalFetch()); + } + private RawLocalFileSystem getRawFs(Configuration conf) { try { return (RawLocalFileSystem) FileSystem.getLocal(conf).getRaw(); diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestShuffleScheduler.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestShuffleScheduler.java index fabfa270d7..b89ffb0ce9 100644 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestShuffleScheduler.java +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestShuffleScheduler.java @@ -55,6 +55,7 @@ import org.apache.tez.runtime.library.api.TezRuntimeConfiguration; import org.apache.tez.runtime.library.common.CompositeInputAttemptIdentifier; import org.apache.tez.runtime.library.common.InputAttemptIdentifier; +import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -248,8 +249,8 @@ public void _testReducerHealth_1(Configuration conf) throws IOException { for (int i = 100; i < 199; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); } @@ -257,9 +258,8 @@ public void _testReducerHealth_1(Configuration conf) throws IOException { new InputAttemptIdentifier(200, 0, "attempt_"); //Should fail here and report exception as reducer is not healthy - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (200 % - totalProducerNodes), - 10000, 200, 1), false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (200 % totalProducerNodes), 10000, 200, 1), false, true); int minFailurePerHost = conf.getInt( TezRuntimeConfiguration.TEZ_RUNTIME_SHUFFLE_MIN_FAILURES_PER_HOST, @@ -330,8 +330,8 @@ public void testReducerHealth_2() throws IOException, InterruptedException { for (int i = 190; i < 200; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); } //Shuffle has not stalled. so no issues. @@ -342,9 +342,8 @@ public void testReducerHealth_2() throws IOException, InterruptedException { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(190, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + - (190 % totalProducerNodes), - 10000, 190, 1), false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (190 % totalProducerNodes), 10000, 190, 1), false, true); //Even when it is stalled, need (320 - 300 = 20) * 3 = 60 failures verify(scheduler.reporter, times(0)).reportException(any(Throwable.class)); @@ -355,16 +354,17 @@ public void testReducerHealth_2() throws IOException, InterruptedException { for (int i = 190; i < 200; i++) { inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); + InputAttemptFetchFailure failure = InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true); } assertEquals(61, scheduler.failedShufflesSinceLastCompletion); @@ -376,12 +376,14 @@ public void testReducerHealth_2() throws IOException, InterruptedException { for (int i = 110; i < 120; i++) { inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); + InputAttemptFetchFailure failure = + InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); } // Should fail now due to fetcherHealthy. (stall has already happened and @@ -432,8 +434,8 @@ public void testReducerHealth_3() throws IOException { //1 fails (last fetch) InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(319, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % totalProducerNodes), - 10000, 319, 1), false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), false, true); //stall the shuffle scheduler.lastProgressTime = System.currentTimeMillis() - 1000000; @@ -441,15 +443,13 @@ public void testReducerHealth_3() throws IOException { assertEquals(scheduler.remainingMaps.get(), 1); //Retry for 3 more times - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % - totalProducerNodes), - 10000, 319, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % - totalProducerNodes), - 10000, 310, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % - totalProducerNodes), - 10000, 310, 1), false, true, false); + InputAttemptFetchFailure failure = InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); + scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 310, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 310, 1), + false, true); // failedShufflesSinceLastCompletion has crossed the limits. Throw error verify(shuffle, times(0)).reportException(any(Throwable.class)); @@ -487,15 +487,15 @@ public void testReducerHealth_4() throws IOException { for (int i = 0; i < 64; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); + InputAttemptFetchFailure failure = + InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % - totalProducerNodes), 10000, i, 1), false, true, false); - - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % - totalProducerNodes), 10000, i, 1), false, true, false); - - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % - totalProducerNodes), 10000, i, 1), false, true, false); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); MapOutput mapOutput = MapOutput .createMemoryMapOutput(inputAttemptIdentifier, mock(FetchedInputAllocatorOrderedGrouped.class), @@ -518,8 +518,8 @@ public void testReducerHealth_4() throws IOException { //1 fails (last fetch) InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(319, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % totalProducerNodes), - 10000, 319, 1), false, true, false); + scheduler.copyFailed(new InputAttemptFetchFailure(inputAttemptIdentifier), + new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), false, true); //stall the shuffle (but within limits) scheduler.lastProgressTime = System.currentTimeMillis() - 100000; @@ -527,15 +527,13 @@ public void testReducerHealth_4() throws IOException { assertEquals(scheduler.remainingMaps.get(), 1); //Retry for 3 more times - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % - totalProducerNodes), - 10000, 319, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % - totalProducerNodes), - 10000, 319, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % - totalProducerNodes), - 10000, 319, 1), false, true, false); + InputAttemptFetchFailure failure = InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); + scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), + false, true); // failedShufflesSinceLastCompletion has crossed the limits. 20% of other nodes had failures as // well. However, it has failed only in one host. So this should proceed @@ -544,9 +542,8 @@ public void testReducerHealth_4() throws IOException { //stall the shuffle (but within limits) scheduler.lastProgressTime = System.currentTimeMillis() - 300000; - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % - totalProducerNodes), - 10000, 319, 1), false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), false, true); verify(shuffle, times(1)).reportException(any(Throwable.class)); } @@ -592,8 +589,9 @@ public void testReducerHealth_5() throws IOException { //1 fails (last fetch) InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(318, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (318 % totalProducerNodes), - 10000, 318, 1), false, true, false); + InputAttemptFetchFailure failure = new InputAttemptFetchFailure(inputAttemptIdentifier); + scheduler.copyFailed(failure, new MapHost("host" + (318 % totalProducerNodes), + 10000, 318, 1), false, true); //stall the shuffle scheduler.lastProgressTime = System.currentTimeMillis() - 1000000; @@ -601,15 +599,12 @@ public void testReducerHealth_5() throws IOException { assertEquals(scheduler.remainingMaps.get(), 1); //Retry for 3 more times - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (318 % - totalProducerNodes), - 10000, 318, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (318 % - totalProducerNodes), - 10000, 318, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (318 % - totalProducerNodes), - 10000, 318, 1), false, true, false); + scheduler.copyFailed(failure, new MapHost("host" + (318 % totalProducerNodes), 10000, 318, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (318 % totalProducerNodes), 10000, 318, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (318 % totalProducerNodes), 10000, 318, 1), + false, true); //Shuffle has not received the events completely. So do not bail out yet. verify(shuffle, times(0)).reportException(any(Throwable.class)); @@ -672,8 +667,8 @@ public void _testReducerHealth_6(Configuration conf) throws IOException { for (int i = 10; i < 15; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); } assertTrue(scheduler.failureCounts.size() >= 5); @@ -686,10 +681,10 @@ public void _testReducerHealth_6(Configuration conf) throws IOException { for (int i = 10; i < 15; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); } boolean checkFailedFetchSinceLastCompletion = conf.getBoolean @@ -749,18 +744,15 @@ public void testReducerHealth_7() throws IOException { for (int i = 100; i < 199; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(inputAttemptIdentifier, - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true, false); - scheduler.copyFailed(inputAttemptIdentifier, - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true, false); + InputAttemptFetchFailure failure = InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); + scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true); } verify(shuffle, atLeast(1)).reportException(any(Throwable.class)); @@ -799,7 +791,8 @@ public void testPenalty() throws IOException, InterruptedException { MapHost mapHost = scheduler.pendingHosts.iterator().next(); //Fails to pull from host0. host0 should be added to penalties - scheduler.copyFailed(inputAttemptIdentifier, mapHost, false, true, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), mapHost, + false, true); //Should not get host, as it is added to penalty loop MapHost host = scheduler.getHost(); @@ -993,7 +986,8 @@ public Void call() throws Exception { } for (int i = 0; i < 10; i++) { - scheduler.copyFailed(identifiers[0], mapHosts[0], false, false, false); + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(identifiers[0]), mapHosts[0], false, + false); } ShuffleScheduler.Penalty[] penaltyArray = new ShuffleScheduler.Penalty[scheduler.getPenalties().size()]; scheduler.getPenalties().toArray(penaltyArray); diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/testutils/RuntimeTestUtils.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/testutils/RuntimeTestUtils.java new file mode 100644 index 0000000000..0885178ee5 --- /dev/null +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/testutils/RuntimeTestUtils.java @@ -0,0 +1,44 @@ +/** + * 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.tez.runtime.library.testutils; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.InputStream; + +import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.ShuffleHeader; + +public final class RuntimeTestUtils { + + private RuntimeTestUtils() { + } + + public static DataInputStream shuffleHeaderToDataInput(ShuffleHeader header) throws IOException { + ByteArrayOutputStream byteOutput = new ByteArrayOutputStream(1000); + DataOutputStream output = new DataOutputStream(byteOutput); + header.write(output); + + InputStream inputStream = new ByteArrayInputStream(byteOutput.toByteArray()); + DataInputStream input = new DataInputStream(inputStream); + + return input; + } +} From 6a6f5b12a70a7feb0935e7ece9c44726f997fd40 Mon Sep 17 00:00:00 2001 From: Bodor Laszlo Date: Tue, 2 Nov 2021 18:26:47 +0100 Subject: [PATCH 4/9] ShuffleScheduler should try to report the original exception (when shuffle becomes unhealthy) (#155) (Laszlo Bodor reviewed by Rajesh Balamohan) (cherry picked from commit 6863a2d9923ed1633d44c708631e454303503d46) --- .../common/shuffle/InputAttemptFetchFailure.java | 10 ++++++++++ .../orderedgrouped/FetcherOrderedGrouped.java | 4 ++-- .../shuffle/orderedgrouped/ShuffleScheduler.java | 12 +++++++----- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java index d94db35c2f..4ce1699cf5 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java @@ -33,6 +33,7 @@ public class InputAttemptFetchFailure { private final InputAttemptIdentifier inputAttemptIdentifier; private final boolean isLocalFetch; private final boolean isDiskErrorAtSource; + private Throwable cause = null; public InputAttemptFetchFailure(InputAttemptIdentifier inputAttemptIdentifier) { this(inputAttemptIdentifier, false, false); @@ -112,4 +113,13 @@ public String toString() { return String.format("%s, isLocalFetch: %s, isDiskErrorAtSource: %s", inputAttemptIdentifier.toString(), isLocalFetch, isDiskErrorAtSource); } + + public InputAttemptFetchFailure withCause(Throwable throwable) { + this.cause = throwable; + return this; + } + + public Throwable getCause() { + return cause; + } } diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java index aaa3a3b217..1f9d8f8c9d 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java @@ -378,7 +378,7 @@ boolean setupConnection(MapHost host, Collection attempt for (InputAttemptIdentifier left : remaining.values()) { // Need to be handling temporary glitches .. // Report read error to the AM to trigger source failure heuristics - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(left), host, connectSucceeded, + scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(left).withCause(ie), host, connectSucceeded, !connectSucceeded); } return false; @@ -738,7 +738,7 @@ protected void setupLocalDiskFetch(MapHost host) throws InterruptedException { if (!stopped) { hasFailures = true; ioErrs.increment(1); - scheduler.copyFailed(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttemptId), + scheduler.copyFailed(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttemptId).withCause(e), host, true, false); LOG.warn("Failed to read local disk output of " + srcAttemptId + " from " + host.getHostIdentifier(), e); diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java index a16d0db162..8758caf830 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java @@ -178,6 +178,7 @@ enum ShuffleErrors { private final Referee referee; @VisibleForTesting final Map failureCounts = new HashMap(); + final Set uniqueHosts = Sets.newHashSet(); private final Map hostFailures = new HashMap(); private final InputContext inputContext; @@ -791,7 +792,7 @@ public synchronized void copyFailed(InputAttemptFetchFailure fetchFailure, MapHo } //Restart consumer in case shuffle is not healthy - if (!isShuffleHealthy(fetchFailure.getInputAttemptIdentifier())) { + if (!isShuffleHealthy(fetchFailure)) { return; } @@ -1005,8 +1006,8 @@ private boolean isFetcherHealthy(String logContext) { return fetcherHealthy; } - boolean isShuffleHealthy(InputAttemptIdentifier srcAttempt) { - + boolean isShuffleHealthy(InputAttemptFetchFailure fetchFailure) { + InputAttemptIdentifier srcAttempt = fetchFailure.getInputAttemptIdentifier(); if (isAbortLimitExceeedFor(srcAttempt)) { return false; } @@ -1048,14 +1049,15 @@ boolean isShuffleHealthy(InputAttemptIdentifier srcAttempt) { + ", pendingInputs=" + (numInputs - doneMaps) + ", fetcherHealthy=" + fetcherHealthy + ", reducerProgressedEnough=" + reducerProgressedEnough - + ", reducerStalled=" + reducerStalled) + + ", reducerStalled=" + reducerStalled + + ", hostFailures=" + hostFailures) + "]"; LOG.error(errorMsg); if (LOG.isDebugEnabled()) { LOG.debug("Host failures=" + hostFailures.keySet()); } // Shuffle knows how to deal with failures post shutdown via the onFailure hook - exceptionReporter.reportException(new IOException(errorMsg)); + exceptionReporter.reportException(new IOException(errorMsg, fetchFailure.getCause())); return false; } return true; From 0917c39c710f72368c374abbe1bd45e679cd8bf7 Mon Sep 17 00:00:00 2001 From: Mark Bathori Date: Fri, 4 Feb 2022 08:40:29 +0100 Subject: [PATCH 5/9] Missing test changes --- .../tez/runtime/api/events/InputReadErrorEvent.java | 4 ++++ .../apache/tez/dag/app/dag/impl/TestTaskAttempt.java | 10 +++++----- .../src/test/java/org/apache/tez/test/TestInput.java | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java b/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java index 3fa6551212..bfb6960ba6 100644 --- a/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java +++ b/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java @@ -70,6 +70,10 @@ public static InputReadErrorEvent create(String diagnostics, int index, return new InputReadErrorEvent(diagnostics, index, version, isLocalFetch, isDiskErrorAtSource); } + public static InputReadErrorEvent create(String diagnostics, int index, int version) { + return create(diagnostics, index, version, false, false); + } + public String getDiagnostics() { return diagnostics; } diff --git a/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java b/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java index 6862bec2ee..e84ae3dbb2 100644 --- a/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java +++ b/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java @@ -1859,7 +1859,7 @@ taListener, taskConf, new SystemClock(), long finishTime = finishEvent.getFinishTime(); verifyEventType(arg.getAllValues(), TaskEventTAUpdate.class, 2); - InputReadErrorEvent mockReEvent = InputReadErrorEvent.create("", 0, 1); + InputReadErrorEvent mockReEvent = InputReadErrorEvent.create("", 0, 1, false, false); EventMetaData mockMeta = mock(EventMetaData.class); TezTaskAttemptID mockDestId1 = mock(TezTaskAttemptID.class); when(mockMeta.getTaskAttemptID()).thenReturn(mockDestId1); @@ -1950,7 +1950,7 @@ taListener, taskConf, new SystemClock(), TaskAttemptState.SUCCEEDED); verify(mockHeartbeatHandler).unregister(taskAttemptID2); - mockReEvent = InputReadErrorEvent.create("", 1, 1); + mockReEvent = InputReadErrorEvent.create("", 1, 1, false, false); mockMeta = mock(EventMetaData.class); mockDestId1 = mock(TezTaskAttemptID.class); when(mockDestId1.getTaskID()).thenReturn(destTaskID); @@ -1992,7 +1992,7 @@ taListener, taskConf, new SystemClock(), TaskAttemptState.SUCCEEDED); verify(mockHeartbeatHandler).unregister(taskAttemptID3); - mockReEvent = InputReadErrorEvent.create("", 1, 1); + mockReEvent = InputReadErrorEvent.create("", 1, 1, false, false); mockMeta = mock(EventMetaData.class); mockDestId1 = mock(TezTaskAttemptID.class); when(mockDestId1.getTaskID()).thenReturn(destTaskID); @@ -2172,12 +2172,12 @@ private void testMapTaskFailingForFetchFailureType(boolean isLocalFetch, TezTaskID taskID = TezTaskID.getInstance(TezVertexID.getInstance(TezDAGID.getInstance("1", 1, 1), 1), 1); TaskAttemptImpl sourceAttempt = new MockTaskAttemptImpl(taskID, 1, eventHandler, null, - new Configuration(), SystemClock.getInstance(), mock(TaskHeartbeatHandler.class), appCtx, + new Configuration(), new SystemClock(), mock(TaskHeartbeatHandler.class), appCtx, false, null, null, false); // the original read error event, sent by reducer task InputReadErrorEvent inputReadErrorEvent = - InputReadErrorEvent.create("", 0, 1, 1, isLocalFetch, isDiskErrorAtSource); + InputReadErrorEvent.create("", 0, 1, isLocalFetch, isDiskErrorAtSource); TezTaskAttemptID destTaskAttemptId = mock(TezTaskAttemptID.class); when(destTaskAttemptId.getTaskID()).thenReturn(mock(TezTaskID.class)); when(destTaskAttemptId.getTaskID().getVertexID()).thenReturn(mock(TezVertexID.class)); diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestInput.java b/tez-tests/src/test/java/org/apache/tez/test/TestInput.java index 811ca3cc17..d6ce6ede7b 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestInput.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestInput.java @@ -263,7 +263,7 @@ public int doRead() { getContext().getUniqueIdentifier() + " index: " + index + " version: " + sourceInputVersion; LOG.info(msg); - events.add(InputReadErrorEvent.create(msg, index, sourceInputVersion)); + events.add(InputReadErrorEvent.create(msg, index, sourceInputVersion, false, false)); } } } From e22090a6b4a855d693103b643143668c1d0222ec Mon Sep 17 00:00:00 2001 From: Mark Bathori Date: Mon, 7 Feb 2022 22:24:24 +0100 Subject: [PATCH 6/9] Revert unnecessary changes --- .../api/events/InputReadErrorEvent.java | 31 +--- .../shuffle/api/ShuffleHandlerError.java | 27 --- .../common/shuffle/api/package-info.java | 22 --- tez-api/src/main/proto/Events.proto | 2 - .../tez/dag/app/dag/impl/TaskAttemptImpl.java | 9 +- .../tez/dag/app/dag/impl/TestTaskAttempt.java | 55 +----- .../tez/auxservices/ShuffleHandler.java | 53 +----- .../tez/auxservices/TestShuffleHandler.java | 100 +--------- .../apache/tez/runtime/api/impl/TezEvent.java | 9 +- .../library/common/shuffle/Fetcher.java | 63 +++---- .../common/shuffle/FetcherCallback.java | 3 +- .../shuffle/InputAttemptFetchFailure.java | 125 ------------- .../common/shuffle/impl/ShuffleManager.java | 16 +- .../orderedgrouped/FetcherOrderedGrouped.java | 74 +++----- .../shuffle/orderedgrouped/ShuffleHeader.java | 7 - .../orderedgrouped/ShuffleScheduler.java | 55 +++--- .../library/common/shuffle/TestFetcher.java | 54 +----- .../shuffle/impl/TestShuffleManager.java | 3 +- .../shuffle/orderedgrouped/TestFetcher.java | 66 ++----- .../orderedgrouped/TestShuffleScheduler.java | 174 +++++++++--------- .../library/testutils/RuntimeTestUtils.java | 44 ----- .../java/org/apache/tez/test/TestInput.java | 2 +- .../apache/tez/test/TestSecureShuffle.java | 2 + 23 files changed, 220 insertions(+), 776 deletions(-) delete mode 100644 tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/ShuffleHandlerError.java delete mode 100644 tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/package-info.java delete mode 100644 tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java delete mode 100644 tez-runtime-library/src/test/java/org/apache/tez/runtime/library/testutils/RuntimeTestUtils.java diff --git a/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java b/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java index bfb6960ba6..7d2e0d25a8 100644 --- a/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java +++ b/tez-api/src/main/java/org/apache/tez/runtime/api/events/InputReadErrorEvent.java @@ -44,34 +44,17 @@ public final class InputReadErrorEvent extends Event { */ private final int version; - /** - * Whether this input read error is caused while fetching local file. - */ - private final boolean isLocalFetch; - - /** - * Whether this input read error is caused because the fetcher detected a fatal, unrecoverable, - * local file read issue from the shuffle handler. - */ - private final boolean isDiskErrorAtSource; - private InputReadErrorEvent(String diagnostics, int index, - int version, boolean isLocalFetch, boolean isDiskErrorAtSource) { + int version) { super(); this.diagnostics = diagnostics; this.index = index; this.version = version; - this.isLocalFetch = isLocalFetch; - this.isDiskErrorAtSource = isDiskErrorAtSource; } public static InputReadErrorEvent create(String diagnostics, int index, - int version, boolean isLocalFetch, boolean isDiskErrorAtSource) { - return new InputReadErrorEvent(diagnostics, index, version, isLocalFetch, isDiskErrorAtSource); - } - - public static InputReadErrorEvent create(String diagnostics, int index, int version) { - return create(diagnostics, index, version, false, false); + int version) { + return new InputReadErrorEvent(diagnostics, index, version); } public String getDiagnostics() { @@ -86,12 +69,4 @@ public int getVersion() { return version; } - public boolean isLocalFetch() { - return isLocalFetch; - } - - public boolean isDiskErrorAtSource() { - return isDiskErrorAtSource; - } - } diff --git a/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/ShuffleHandlerError.java b/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/ShuffleHandlerError.java deleted file mode 100644 index 09137de673..0000000000 --- a/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/ShuffleHandlerError.java +++ /dev/null @@ -1,27 +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.tez.runtime.library.common.shuffle.api; - -/** - * ShuffleHandlerError enum encapsulates possible error messages that can be propagated from - * ShuffleHandler to fetchers. Depending on the message, fetchers can make better decisions, or give - * AM a hint in order to let it make better decisions in case of shuffle issues. - */ -public enum ShuffleHandlerError { - DISK_ERROR_EXCEPTION -} diff --git a/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/package-info.java b/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/package-info.java deleted file mode 100644 index 9ad8e61d50..0000000000 --- a/tez-api/src/main/java/org/apache/tez/runtime/library/common/shuffle/api/package-info.java +++ /dev/null @@ -1,22 +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. - */ - -@Private -package org.apache.tez.runtime.library.common.shuffle.api; - -import org.apache.hadoop.classification.InterfaceAudience.Private; \ No newline at end of file diff --git a/tez-api/src/main/proto/Events.proto b/tez-api/src/main/proto/Events.proto index e041c33f60..71235004da 100644 --- a/tez-api/src/main/proto/Events.proto +++ b/tez-api/src/main/proto/Events.proto @@ -39,8 +39,6 @@ message InputReadErrorEventProto { optional int32 index = 1; optional string diagnostics = 2; optional int32 version = 3; - optional bool is_local_fetch = 4; - optional bool is_disk_error_at_source = 5; } message InputFailedEventProto { diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java b/tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java index 5394128044..5a691190e9 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java @@ -1769,7 +1769,6 @@ public TaskAttemptStateInternal transition(TaskAttemptImpl attempt, + " at inputIndex " + failedInputIndexOnDestTa); long time = attempt.clock.getTime(); Long firstErrReportTime = attempt.uniquefailedOutputReports.get(failedDestTaId); - if (firstErrReportTime == null) { attempt.uniquefailedOutputReports.put(failedDestTaId, time); firstErrReportTime = time; @@ -1796,8 +1795,7 @@ public TaskAttemptStateInternal transition(TaskAttemptImpl attempt, // If needed we can launch a background task without failing this task // to generate a copy of the output just in case. // If needed we can consider only running consumer tasks - if (!crossTimeDeadline && withinFailureFractionLimits && withinOutputFailureLimits - && !(readErrorEvent.isLocalFetch() || readErrorEvent.isDiskErrorAtSource())) { + if (!crossTimeDeadline && withinFailureFractionLimits && withinOutputFailureLimits) { return attempt.getInternalState(); } String message = attempt.getID() + " being failed for too many output errors. " @@ -1808,10 +1806,7 @@ public TaskAttemptStateInternal transition(TaskAttemptImpl attempt, + ", MAX_ALLOWED_OUTPUT_FAILURES=" + maxAllowedOutputFailures + ", MAX_ALLOWED_TIME_FOR_TASK_READ_ERROR_SEC=" + maxAllowedTimeForTaskReadErrorSec - + ", readErrorTimespan=" + readErrorTimespanSec - + ", isLocalFetch=" + readErrorEvent.isLocalFetch() - + ", isDiskErrorAtSource=" + readErrorEvent.isDiskErrorAtSource(); - + + ", readErrorTimespan=" + readErrorTimespanSec; LOG.info(message); attempt.addDiagnosticInfo(message); // send input failed event diff --git a/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java b/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java index e84ae3dbb2..41cce3b60e 100644 --- a/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java +++ b/tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskAttempt.java @@ -19,6 +19,7 @@ package org.apache.tez.dag.app.dag.impl; import org.apache.tez.dag.app.MockClock; +import org.apache.tez.dag.app.rm.AMSchedulerEventTAStateUpdated; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; @@ -82,7 +83,6 @@ import org.apache.tez.dag.app.TaskCommunicatorWrapper; import org.apache.tez.dag.app.TaskHeartbeatHandler; import org.apache.tez.dag.app.dag.TaskAttemptStateInternal; -import org.apache.tez.dag.app.dag.DAG; import org.apache.tez.dag.app.dag.Task; import org.apache.tez.dag.app.dag.Vertex; import org.apache.tez.dag.app.dag.event.DAGEvent; @@ -127,7 +127,6 @@ import org.junit.BeforeClass; import org.junit.Test; import org.mockito.ArgumentCaptor; -import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -1859,7 +1858,7 @@ taListener, taskConf, new SystemClock(), long finishTime = finishEvent.getFinishTime(); verifyEventType(arg.getAllValues(), TaskEventTAUpdate.class, 2); - InputReadErrorEvent mockReEvent = InputReadErrorEvent.create("", 0, 1, false, false); + InputReadErrorEvent mockReEvent = InputReadErrorEvent.create("", 0, 1); EventMetaData mockMeta = mock(EventMetaData.class); TezTaskAttemptID mockDestId1 = mock(TezTaskAttemptID.class); when(mockMeta.getTaskAttemptID()).thenReturn(mockDestId1); @@ -1950,7 +1949,7 @@ taListener, taskConf, new SystemClock(), TaskAttemptState.SUCCEEDED); verify(mockHeartbeatHandler).unregister(taskAttemptID2); - mockReEvent = InputReadErrorEvent.create("", 1, 1, false, false); + mockReEvent = InputReadErrorEvent.create("", 1, 1); mockMeta = mock(EventMetaData.class); mockDestId1 = mock(TezTaskAttemptID.class); when(mockDestId1.getTaskID()).thenReturn(destTaskID); @@ -1992,7 +1991,7 @@ taListener, taskConf, new SystemClock(), TaskAttemptState.SUCCEEDED); verify(mockHeartbeatHandler).unregister(taskAttemptID3); - mockReEvent = InputReadErrorEvent.create("", 1, 1, false, false); + mockReEvent = InputReadErrorEvent.create("", 1, 1); mockMeta = mock(EventMetaData.class); mockDestId1 = mock(TezTaskAttemptID.class); when(mockDestId1.getTaskID()).thenReturn(destTaskID); @@ -2155,52 +2154,6 @@ taListener, taskConf, new SystemClock(), Assert.assertEquals(1, taImpl.taskAttemptFinishedEventLogged); } - @Test - public void testMapTaskIsBlamedImmediatelyOnLocalFetchFailure() throws ServicePluginException { - // local fetch failure or disk read error at source -> turn source attempt to FAIL_IN_PROGRESS - testMapTaskFailingForFetchFailureType(true, true, TaskAttemptStateInternal.FAIL_IN_PROGRESS); - testMapTaskFailingForFetchFailureType(true, false, TaskAttemptStateInternal.FAIL_IN_PROGRESS); - testMapTaskFailingForFetchFailureType(false, true, TaskAttemptStateInternal.FAIL_IN_PROGRESS); - - // remote fetch failure -> won't change current state - testMapTaskFailingForFetchFailureType(false, false, TaskAttemptStateInternal.NEW); - } - - private void testMapTaskFailingForFetchFailureType(boolean isLocalFetch, - boolean isDiskErrorAtSource, TaskAttemptStateInternal expectedState) { - EventHandler eventHandler = mock(EventHandler.class); - TezTaskID taskID = - TezTaskID.getInstance(TezVertexID.getInstance(TezDAGID.getInstance("1", 1, 1), 1), 1); - TaskAttemptImpl sourceAttempt = new MockTaskAttemptImpl(taskID, 1, eventHandler, null, - new Configuration(), new SystemClock(), mock(TaskHeartbeatHandler.class), appCtx, - false, null, null, false); - - // the original read error event, sent by reducer task - InputReadErrorEvent inputReadErrorEvent = - InputReadErrorEvent.create("", 0, 1, isLocalFetch, isDiskErrorAtSource); - TezTaskAttemptID destTaskAttemptId = mock(TezTaskAttemptID.class); - when(destTaskAttemptId.getTaskID()).thenReturn(mock(TezTaskID.class)); - when(destTaskAttemptId.getTaskID().getVertexID()).thenReturn(mock(TezVertexID.class)); - when(appCtx.getCurrentDAG()).thenReturn(mock(DAG.class)); - when(appCtx.getCurrentDAG().getVertex(Mockito.any(TezVertexID.class))) - .thenReturn(mock(Vertex.class)); - when(appCtx.getCurrentDAG().getVertex(Mockito.any(TezVertexID.class)).getRunningTasks()) - .thenReturn(100); - - EventMetaData mockMeta = mock(EventMetaData.class); - when(mockMeta.getTaskAttemptID()).thenReturn(destTaskAttemptId); - TezEvent tezEvent = new TezEvent(inputReadErrorEvent, mockMeta); - - // the event is propagated to map task's event handler - TaskAttemptEventOutputFailed outputFailedEvent = - new TaskAttemptEventOutputFailed(sourceAttempt.getID(), tezEvent, 11); - - Assert.assertEquals(TaskAttemptStateInternal.NEW, sourceAttempt.getInternalState()); - TaskAttemptStateInternal resultState = new TaskAttemptImpl.OutputReportedFailedTransition() - .transition(sourceAttempt, outputFailedEvent); - Assert.assertEquals(expectedState, resultState); - } - private Event verifyEventType(List events, Class eventClass, int expectedOccurences) { int count = 0; diff --git a/tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java b/tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java index fb28a0f4b1..1bdddde529 100644 --- a/tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java +++ b/tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java @@ -71,7 +71,6 @@ import org.apache.tez.common.security.JobTokenSecretManager; import org.apache.tez.runtime.library.common.Constants; import org.apache.tez.runtime.library.common.security.SecureShuffleUtils; -import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.ShuffleHeader; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.annotation.Metric; @@ -85,7 +84,6 @@ import org.apache.hadoop.security.token.Token; import org.apache.tez.runtime.library.common.sort.impl.TezIndexRecord; import org.apache.hadoop.util.Shell; -import org.apache.hadoop.util.DiskChecker.DiskErrorException; import org.apache.hadoop.yarn.api.records.ApplicationId; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.proto.YarnServerCommonProtos.VersionProto; @@ -646,10 +644,6 @@ protected Shuffle getShuffle(Configuration conf) { return new Shuffle(conf); } - protected JobTokenSecretManager getSecretManager() { - return secretManager; - } - private void recoverState(Configuration conf) throws IOException { Path recoveryRoot = getRecoveryPath(); if (recoveryRoot != null) { @@ -769,7 +763,7 @@ private void checkVersion() throws IOException { private void addJobToken(JobID jobId, String user, Token jobToken) { userRsrc.put(jobId.toString(), user); - getSecretManager().addTokenForJob(jobId.toString(), jobToken); + secretManager.addTokenForJob(jobId.toString(), jobToken); LOG.info("Added token for " + jobId.toString()); } @@ -815,7 +809,7 @@ private void recordJobShuffleInfo(JobID jobId, String user, private void removeJobShuffleInfo(JobID jobId) throws IOException { String jobIdStr = jobId.toString(); - getSecretManager().removeTokenForJob(jobIdStr); + secretManager.removeTokenForJob(jobIdStr); userRsrc.remove(jobIdStr); if (stateDb != null) { try { @@ -1080,19 +1074,11 @@ private void handleRequest(ChannelHandlerContext ctx, FullHttpRequest request) try { populateHeaders(mapIds, jobId, dagId, user, reduceRange, response, keepAliveParam, mapOutputInfoMap); - } catch (DiskErrorException e) { // fatal error: fetcher should be aware of that - LOG.error("Shuffle error in populating headers (fatal: DiskErrorException):", e); - String errorMessage = getErrorMessage(e); - // custom message, might be noticed by fetchers - // it should reuse the current response object, as headers have been already set for it - sendFakeShuffleHeaderWithError(ctx, - ShuffleHandlerError.DISK_ERROR_EXCEPTION + ": " + errorMessage, response); - return; - } catch (IOException e) { + } catch(IOException e) { ch.write(response); LOG.error("Shuffle error in populating headers :", e); String errorMessage = getErrorMessage(e); - sendError(ctx, errorMessage, INTERNAL_SERVER_ERROR); + sendError(ctx,errorMessage , INTERNAL_SERVER_ERROR); return; } ch.write(response); @@ -1343,7 +1329,7 @@ public void finish() { protected void verifyRequest(String appid, ChannelHandlerContext ctx, HttpRequest request, HttpResponse response, URL requestUri) throws IOException { - SecretKey tokenSecret = getSecretManager().retrieveTokenSecret(appid); + SecretKey tokenSecret = secretManager.retrieveTokenSecret(appid); if (null == tokenSecret) { LOG.info("Request for unknown token " + appid); throw new IOException("could not find jobid"); @@ -1439,46 +1425,25 @@ protected ChannelFuture sendMapOutput(ChannelHandlerContext ctx, Channel ch, return writeFuture; } - protected void sendError(ChannelHandlerContext ctx, HttpResponseStatus status) { + protected void sendError(ChannelHandlerContext ctx, + HttpResponseStatus status) { sendError(ctx, "", status); } protected void sendError(ChannelHandlerContext ctx, String message, HttpResponseStatus status) { FullHttpResponse response = new DefaultFullHttpResponse(HTTP_1_1, status); - sendError(ctx, message, response); - response.release(); - } - protected void sendError(ChannelHandlerContext ctx, String message, FullHttpResponse response) { - sendError(ctx, Unpooled.copiedBuffer(message, CharsetUtil.UTF_8), response); - } - - private void sendFakeShuffleHeaderWithError(ChannelHandlerContext ctx, String message, - HttpResponse response) throws IOException { - FullHttpResponse fullResponse = - new DefaultFullHttpResponse(response.getProtocolVersion(), response.getStatus()); - fullResponse.headers().set(response.headers()); - - ShuffleHeader header = new ShuffleHeader(message, -1, -1, -1); - DataOutputBuffer out = new DataOutputBuffer(); - header.write(out); - - sendError(ctx, wrappedBuffer(out.getData(), 0, out.getLength()), fullResponse); - fullResponse.release(); - } - - protected void sendError(ChannelHandlerContext ctx, ByteBuf content, - FullHttpResponse response) { response.headers().set(CONTENT_TYPE, "text/plain; charset=UTF-8"); // Put shuffle version into http header response.headers().set(ShuffleHeader.HTTP_HEADER_NAME, ShuffleHeader.DEFAULT_HTTP_HEADER_NAME); response.headers().set(ShuffleHeader.HTTP_HEADER_VERSION, ShuffleHeader.DEFAULT_HTTP_HEADER_VERSION); - response.content().writeBytes(content); + response.content().writeBytes(Unpooled.copiedBuffer(message, CharsetUtil.UTF_8)); // Close the connection as soon as the error message is sent. ctx.channel().writeAndFlush(response).addListener(ChannelFutureListener.CLOSE); + response.release(); } @Override diff --git a/tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java b/tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java index e27ff7c4ef..2bf9cb2ad0 100644 --- a/tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java +++ b/tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java @@ -20,7 +20,6 @@ //import static org.apache.hadoop.test.MetricsAsserts.assertCounter; //import static org.apache.hadoop.test.MetricsAsserts.assertGauge; //import static org.apache.hadoop.test.MetricsAsserts.getMetrics; -import org.apache.hadoop.util.DiskChecker.DiskErrorException; import static org.junit.Assert.assertTrue; import static io.netty.buffer.Unpooled.wrappedBuffer; import static org.junit.Assert.assertEquals; @@ -40,6 +39,7 @@ import java.net.URL; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; @@ -48,6 +48,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; +import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.fs.Path; @@ -58,10 +59,6 @@ import org.apache.tez.runtime.library.common.security.SecureShuffleUtils; import org.apache.tez.common.security.JobTokenIdentifier; import org.apache.tez.common.security.JobTokenSecretManager; -import org.apache.tez.http.BaseHttpConnection; -import org.apache.tez.http.HttpConnectionParams; -import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; -import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.ShuffleHeader; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.impl.MetricsSystemImpl; @@ -172,52 +169,6 @@ protected boolean isSocketKeepAlive() { } } - class MockShuffleHandlerWithFatalDiskError extends org.apache.tez.auxservices.ShuffleHandler { - public static final String MESSAGE = - "Could not find application_1234/240/output/attempt_1234_0/file.out.index"; - - private JobTokenSecretManager secretManager = - new JobTokenSecretManager(JobTokenSecretManager.createSecretKey(getSecret().getBytes())); - - protected JobTokenSecretManager getSecretManager(){ - return secretManager; - } - - @Override - protected Shuffle getShuffle(final Configuration conf) { - return new Shuffle(conf) { - @Override - protected void verifyRequest(String appid, ChannelHandlerContext ctx, HttpRequest request, - HttpResponse response, URL requestUri) throws IOException { - super.verifyRequest(appid, ctx, request, response, requestUri); - } - - @Override - protected MapOutputInfo getMapOutputInfo(String dagId, String mapId, Range reduceRange, - String jobId, String user) { - return null; - } - - @Override - protected void populateHeaders(List mapIds, String jobId, String dagId, String user, - Range reduceRange, HttpResponse response, boolean keepAliveParam, - Map infoMap) throws IOException { - throw new DiskErrorException(MESSAGE); - } - - @Override - protected ChannelFuture sendMapOutput(ChannelHandlerContext ctx, Channel ch, String user, - String mapId, Range reduceRange, MapOutputInfo info) throws IOException { - return null; - } - }; - } - - public String getSecret() { - return "secret"; - } - } - /** * Test the validation of ShuffleHandler's meta-data's serialization and * de-serialization. @@ -1359,53 +1310,6 @@ public void testSendMapCount() throws Exception { sh.close(); } - @Test - public void testShuffleHandlerSendsDiskError() throws Exception { - Configuration conf = new Configuration(); - conf.setInt(ShuffleHandler.SHUFFLE_PORT_CONFIG_KEY, 0); - - DataInputStream input = null; - MockShuffleHandlerWithFatalDiskError shuffleHandler = - new MockShuffleHandlerWithFatalDiskError(); - try { - shuffleHandler.init(conf); - shuffleHandler.start(); - - String shuffleBaseURL = "http://127.0.0.1:" - + shuffleHandler.getConfig().get(ShuffleHandler.SHUFFLE_PORT_CONFIG_KEY); - URL url = new URL( - shuffleBaseURL + "/mapOutput?job=job_12345_1&dag=1&reduce=1&map=attempt_12345_1_m_1_0"); - shuffleHandler.secretManager.addTokenForJob("job_12345_1", - new Token<>("id".getBytes(), shuffleHandler.getSecret().getBytes(), null, null)); - - HttpConnectionParams httpConnectionParams = ShuffleUtils.getHttpConnectionParams(conf); - BaseHttpConnection httpConnection = ShuffleUtils.getHttpConnection(true, url, - httpConnectionParams, "testFetcher", shuffleHandler.secretManager); - - boolean connectSucceeded = httpConnection.connect(); - Assert.assertTrue(connectSucceeded); - - input = httpConnection.getInputStream(); - httpConnection.validate(); - - ShuffleHeader header = new ShuffleHeader(); - header.readFields(input); - - // message is encoded in the shuffle header, and can be checked by fetchers - Assert.assertEquals( - ShuffleHandlerError.DISK_ERROR_EXCEPTION + ": " + MockShuffleHandlerWithFatalDiskError.MESSAGE, - header.getMapId()); - Assert.assertEquals(-1, header.getCompressedLength()); - Assert.assertEquals(-1, header.getUncompressedLength()); - Assert.assertEquals(-1, header.getPartition()); - } finally { - if (input != null) { - input.close(); - } - shuffleHandler.close(); - } - } - public ChannelFuture createMockChannelFuture(Channel mockCh, final List listenerList) { final ChannelFuture mockFuture = mock(ChannelFuture.class); diff --git a/tez-runtime-internals/src/main/java/org/apache/tez/runtime/api/impl/TezEvent.java b/tez-runtime-internals/src/main/java/org/apache/tez/runtime/api/impl/TezEvent.java index ebea9a4f3f..e7af4a1ebe 100644 --- a/tez-runtime-internals/src/main/java/org/apache/tez/runtime/api/impl/TezEvent.java +++ b/tez-runtime-internals/src/main/java/org/apache/tez/runtime/api/impl/TezEvent.java @@ -191,8 +191,6 @@ private void serializeEvent(DataOutput out) throws IOException { .setIndex(ideEvt.getIndex()) .setDiagnostics(ideEvt.getDiagnostics()) .setVersion(ideEvt.getVersion()) - .setIsLocalFetch(ideEvt.isLocalFetch()) - .setIsDiskErrorAtSource(ideEvt.isDiskErrorAtSource()) .build(); break; case TASK_ATTEMPT_FAILED_EVENT: @@ -296,9 +294,10 @@ private void deserializeEvent(DataInput in) throws IOException { event = ProtoConverters.convertVertexManagerEventFromProto(vmProto); break; case INPUT_READ_ERROR_EVENT: - InputReadErrorEventProto ideProto = InputReadErrorEventProto.parseFrom(input); - event = InputReadErrorEvent.create(ideProto.getDiagnostics(), ideProto.getIndex(), - ideProto.getVersion(), ideProto.getIsLocalFetch(), ideProto.getIsDiskErrorAtSource()); + InputReadErrorEventProto ideProto = + InputReadErrorEventProto.parseFrom(input); + event = InputReadErrorEvent.create(ideProto.getDiagnostics(), + ideProto.getIndex(), ideProto.getVersion()); break; case TASK_ATTEMPT_FAILED_EVENT: TaskAttemptFailedEventProto tfProto = diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/Fetcher.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/Fetcher.java index 7f2033f976..f2412e35d9 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/Fetcher.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/Fetcher.java @@ -66,7 +66,7 @@ import org.apache.tez.runtime.library.common.sort.impl.TezSpillRecord; import org.apache.tez.runtime.library.exceptions.FetcherReadTimeoutException; import org.apache.tez.runtime.library.common.shuffle.FetchedInput.Type; -import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; + import org.apache.tez.common.Preconditions; /** @@ -277,8 +277,7 @@ public FetchResult callInternal() throws Exception { HostFetchResult hostFetchResult; - boolean isLocalFetch = localDiskFetchEnabled && host.equals(localHostname) && port == shufflePort; - if (isLocalFetch) { + if (localDiskFetchEnabled && host.equals(localHostname) && port == shufflePort) { hostFetchResult = setupLocalDiskFetch(); } else if (multiplex) { hostFetchResult = doSharedFetch(); @@ -289,7 +288,7 @@ public FetchResult callInternal() throws Exception { if (hostFetchResult.failedInputs != null && hostFetchResult.failedInputs.length > 0) { if (!isShutDown.get()) { LOG.warn("copyInputs failed for tasks " + Arrays.toString(hostFetchResult.failedInputs)); - for (InputAttemptFetchFailure left : hostFetchResult.failedInputs) { + for (InputAttemptIdentifier left : hostFetchResult.failedInputs) { fetcherCallback.fetchFailed(host, left, hostFetchResult.connectFailed); } } else { @@ -505,7 +504,7 @@ private HostFetchResult setupConnection(Collection attem // ioErrs.increment(1); // If connect did not succeed, just mark all the maps as failed, // indirectly penalizing the host - InputAttemptFetchFailure[] failedFetches = null; + InputAttemptIdentifier[] failedFetches = null; if (isShutDown.get()) { if (isDebugEnabled) { LOG.debug( @@ -513,7 +512,8 @@ private HostFetchResult setupConnection(Collection attem e.getClass().getName() + ", Message: " + e.getMessage()); } } else { - failedFetches = InputAttemptFetchFailure.fromAttempts(srcAttemptsRemaining.values()); + failedFetches = srcAttemptsRemaining.values(). + toArray(new InputAttemptIdentifier[srcAttemptsRemaining.values().size()]); } return new HostFetchResult(new FetchResult(host, port, partition, partitionCount, srcAttemptsRemaining.values()), failedFetches, true); } @@ -546,7 +546,7 @@ private HostFetchResult setupConnection(Collection attem LOG.warn("Fetch Failure from host while connecting: " + host + ", attempt: " + firstAttempt + " Informing ShuffleManager: ", e); return new HostFetchResult(new FetchResult(host, port, partition, partitionCount, srcAttemptsRemaining.values()), - new InputAttemptFetchFailure[] { new InputAttemptFetchFailure(firstAttempt) }, false); + new InputAttemptIdentifier[] { firstAttempt }, false); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); //reset status @@ -583,7 +583,7 @@ protected HostFetchResult doHttpFetch(CachingCallBack callback) { // On any error, faildTasks is not null and we exit // after putting back the remaining maps to the // yet_to_be_fetched list and marking the failed tasks. - InputAttemptFetchFailure[] failedInputs = null; + InputAttemptIdentifier[] failedInputs = null; while (!srcAttemptsRemaining.isEmpty() && failedInputs == null) { InputAttemptIdentifier inputAttemptIdentifier = srcAttemptsRemaining.entrySet().iterator().next().getValue(); @@ -710,7 +710,7 @@ public void freeResources(FetchedInput fetchedInput) { } } - InputAttemptFetchFailure[] failedFetches = null; + InputAttemptIdentifier[] failedFetches = null; if (failMissing && srcAttemptsRemaining.size() > 0) { if (isShutDown.get()) { if (isDebugEnabled) { @@ -719,8 +719,8 @@ public void freeResources(FetchedInput fetchedInput) { " remaining inputs"); } } else { - failedFetches = - InputAttemptFetchFailure.fromAttemptsLocalFetchFailure(srcAttemptsRemaining.values()); + failedFetches = srcAttemptsRemaining.values(). + toArray(new InputAttemptIdentifier[srcAttemptsRemaining.values().size()]); } } else { // nothing needs to be done to requeue remaining entries @@ -769,10 +769,10 @@ public Map getPathToAttemptMap() { static class HostFetchResult { private final FetchResult fetchResult; - private final InputAttemptFetchFailure[] failedInputs; + private final InputAttemptIdentifier[] failedInputs; private final boolean connectFailed; - public HostFetchResult(FetchResult fetchResult, InputAttemptFetchFailure[] failedInputs, + public HostFetchResult(FetchResult fetchResult, InputAttemptIdentifier[] failedInputs, boolean connectFailed) { this.fetchResult = fetchResult; this.failedInputs = failedInputs; @@ -830,11 +830,8 @@ public String toString() { return "id: " + srcAttemptId + ", decompressed length: " + decompressedLength + ", compressed length: " + compressedLength + ", reduce: " + forReduce; } } - - @VisibleForTesting - InputAttemptFetchFailure[] fetchInputs(DataInputStream input, CachingCallBack callback, - InputAttemptIdentifier inputAttemptIdentifier) - throws FetcherReadTimeoutException { + private InputAttemptIdentifier[] fetchInputs(DataInputStream input, + CachingCallBack callback, InputAttemptIdentifier inputAttemptIdentifier) throws FetcherReadTimeoutException { FetchedInput fetchedInput = null; InputAttemptIdentifier srcAttemptId = null; long decompressedLength = 0; @@ -858,19 +855,9 @@ InputAttemptFetchFailure[] fetchInputs(DataInputStream input, CachingCallBack ca header.readFields(input); pathComponent = header.getMapId(); if (!pathComponent.startsWith(InputAttemptIdentifier.PATH_PREFIX)) { - if (pathComponent.startsWith(ShuffleHandlerError.DISK_ERROR_EXCEPTION.toString())) { - LOG.warn("Invalid map id: " + header.getMapId() + ", expected to start with " - + InputAttemptIdentifier.PATH_PREFIX + ", partition: " + header.getPartition() - + " while fetching " + inputAttemptIdentifier); - // this should be treated as local fetch failure while reporting later - return new InputAttemptFetchFailure[] { - InputAttemptFetchFailure.fromDiskErrorAtSource(inputAttemptIdentifier) }; - } else { - throw new IllegalArgumentException( - "Invalid map id: " + header.getMapId() + ", expected to start with " - + InputAttemptIdentifier.PATH_PREFIX + ", partition: " + header.getPartition() - + " while fetching " + inputAttemptIdentifier); - } + throw new IllegalArgumentException("Invalid map id: " + header.getMapId() + ", expected to start with " + + InputAttemptIdentifier.PATH_PREFIX + ", partition: " + header.getPartition() + + " while fetching " + inputAttemptIdentifier); } srcAttemptId = pathToAttemptMap.get(new PathPartition(pathComponent, header.getPartition())); @@ -895,7 +882,7 @@ InputAttemptFetchFailure[] fetchInputs(DataInputStream input, CachingCallBack ca if (!isShutDown.get()) { LOG.warn("Invalid src id ", e); // Don't know which one was bad, so consider all of them as bad - return InputAttemptFetchFailure.fromAttempts(srcAttemptsRemaining.values()); + return srcAttemptsRemaining.values().toArray(new InputAttemptIdentifier[srcAttemptsRemaining.size()]); } else { if (isDebugEnabled) { LOG.debug("Already shutdown. Ignoring badId error with message: " + e.getMessage()); @@ -914,8 +901,7 @@ InputAttemptFetchFailure[] fetchInputs(DataInputStream input, CachingCallBack ca srcAttemptId = getNextRemainingAttempt(); } assert (srcAttemptId != null); - return new InputAttemptFetchFailure[] { - InputAttemptFetchFailure.fromAttempt(srcAttemptId) }; + return new InputAttemptIdentifier[]{srcAttemptId}; } else { if (isDebugEnabled) { LOG.debug("Already shutdown. Ignoring verification failure."); @@ -1017,10 +1003,10 @@ InputAttemptFetchFailure[] fetchInputs(DataInputStream input, CachingCallBack ca // Cleanup the fetchedInput before returning. cleanupFetchedInput(fetchedInput); if (srcAttemptId == null) { - return InputAttemptFetchFailure.fromAttempts(srcAttemptsRemaining.values()); + return srcAttemptsRemaining.values() + .toArray(new InputAttemptIdentifier[srcAttemptsRemaining.size()]); } else { - return new InputAttemptFetchFailure[] { - new InputAttemptFetchFailure(srcAttemptId) }; + return new InputAttemptIdentifier[] { srcAttemptId }; } } LOG.warn("Failed to shuffle output of " + srcAttemptId + " from " + host, @@ -1029,8 +1015,7 @@ InputAttemptFetchFailure[] fetchInputs(DataInputStream input, CachingCallBack ca // Cleanup the fetchedInput cleanupFetchedInput(fetchedInput); // metrics.failedFetch(); - return new InputAttemptFetchFailure[] { - new InputAttemptFetchFailure(srcAttemptId) }; + return new InputAttemptIdentifier[] { srcAttemptId }; } return null; } diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/FetcherCallback.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/FetcherCallback.java index b751fb9ce0..34bd272909 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/FetcherCallback.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/FetcherCallback.java @@ -28,7 +28,6 @@ public void fetchSucceeded(String host, InputAttemptIdentifier srcAttemptIdentif FetchedInput fetchedInput, long fetchedBytes, long decompressedLength, long copyDuration) throws IOException; - public void fetchFailed(String host, InputAttemptFetchFailure srcAttemptFetchFailure, - boolean connectFailed); + public void fetchFailed(String host, InputAttemptIdentifier srcAttemptIdentifier, boolean connectFailed); } diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java deleted file mode 100644 index 4ce1699cf5..0000000000 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/InputAttemptFetchFailure.java +++ /dev/null @@ -1,125 +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.tez.runtime.library.common.shuffle; - -import java.util.Arrays; -import java.util.Collection; - -import org.apache.tez.runtime.library.common.CompositeInputAttemptIdentifier; -import org.apache.tez.runtime.library.common.InputAttemptIdentifier; - -/** - * InputAttemptFetchFailure is supposed to wrap an InputAttemptIdentifier with any kind of failure - * information during fetch. It can be useful for propagating as a single object instead of multiple - * parameters (local fetch error, remote fetch error, connect failed, read failed, etc.). - */ -public class InputAttemptFetchFailure { - - private final InputAttemptIdentifier inputAttemptIdentifier; - private final boolean isLocalFetch; - private final boolean isDiskErrorAtSource; - private Throwable cause = null; - - public InputAttemptFetchFailure(InputAttemptIdentifier inputAttemptIdentifier) { - this(inputAttemptIdentifier, false, false); - } - - public InputAttemptFetchFailure(InputAttemptIdentifier inputAttemptIdentifier, - boolean isLocalFetch, boolean isDiskErrorAtSource) { - this.inputAttemptIdentifier = inputAttemptIdentifier; - this.isLocalFetch = isLocalFetch; - this.isDiskErrorAtSource = isDiskErrorAtSource; - } - - public InputAttemptIdentifier getInputAttemptIdentifier() { - return inputAttemptIdentifier; - } - - public boolean isLocalFetch() { - return isLocalFetch; - } - - public boolean isDiskErrorAtSource() { - return isDiskErrorAtSource; - } - - public static InputAttemptFetchFailure fromAttempt(InputAttemptIdentifier attempt) { - return new InputAttemptFetchFailure(attempt, false, false); - } - - public static InputAttemptFetchFailure fromLocalFetchFailure(InputAttemptIdentifier attempt) { - return new InputAttemptFetchFailure(attempt, true, false); - } - - public static InputAttemptFetchFailure fromDiskErrorAtSource(InputAttemptIdentifier attempt) { - return new InputAttemptFetchFailure(attempt, false, true); - } - - public static InputAttemptFetchFailure[] fromAttempts(Collection values) { - return values.stream().map(identifier -> new InputAttemptFetchFailure(identifier, false, false)) - .toArray(InputAttemptFetchFailure[]::new); - } - - public static InputAttemptFetchFailure[] fromAttempts(InputAttemptIdentifier[] values) { - return Arrays.asList(values).stream() - .map(identifier -> new InputAttemptFetchFailure(identifier, false, false)) - .toArray(InputAttemptFetchFailure[]::new); - } - - public static InputAttemptFetchFailure[] fromAttemptsLocalFetchFailure( - Collection values) { - return values.stream().map(identifier -> new InputAttemptFetchFailure(identifier, true, false)) - .toArray(InputAttemptFetchFailure[]::new); - } - - public static InputAttemptFetchFailure fromCompositeAttemptLocalFetchFailure( - CompositeInputAttemptIdentifier compositeInputAttemptIdentifier) { - return new InputAttemptFetchFailure(compositeInputAttemptIdentifier, true, false); - } - - @Override - public boolean equals(Object obj) { - if (obj == null || (obj.getClass() != this.getClass())) { - return false; - } - return inputAttemptIdentifier.equals(((InputAttemptFetchFailure) obj).inputAttemptIdentifier) - && isLocalFetch == ((InputAttemptFetchFailure) obj).isLocalFetch - && isDiskErrorAtSource == ((InputAttemptFetchFailure) obj).isDiskErrorAtSource; - } - - @Override - public int hashCode() { - return 31 * inputAttemptIdentifier.hashCode() + 31 * (isLocalFetch ? 0 : 1) - + 31 * (isDiskErrorAtSource ? 0 : 1); - } - - @Override - public String toString() { - return String.format("%s, isLocalFetch: %s, isDiskErrorAtSource: %s", - inputAttemptIdentifier.toString(), isLocalFetch, isDiskErrorAtSource); - } - - public InputAttemptFetchFailure withCause(Throwable throwable) { - this.cause = throwable; - return this; - } - - public Throwable getCause() { - return cause; - } -} diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java index ff64b7f664..9b65103888 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java @@ -80,7 +80,6 @@ import org.apache.tez.runtime.library.common.shuffle.Fetcher.FetcherBuilder; import org.apache.tez.runtime.library.common.shuffle.FetcherCallback; import org.apache.tez.runtime.library.common.shuffle.HostPort; -import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.InputHost; import org.apache.tez.runtime.library.common.shuffle.InputHost.PartitionToInputs; import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; @@ -792,15 +791,12 @@ private void reportFatalError(Throwable exception, String message) { @Override public void fetchFailed(String host, - InputAttemptFetchFailure inputAttemptFetchFailure, boolean connectFailed) { + InputAttemptIdentifier srcAttemptIdentifier, boolean connectFailed) { // TODO NEWTEZ. Implement logic to report fetch failures after a threshold. // For now, reporting immediately. - InputAttemptIdentifier srcAttemptIdentifier = inputAttemptFetchFailure.getInputAttemptIdentifier(); - LOG.info( - "{}: Fetch failed for src: {} InputIdentifier: {}, connectFailed: {}, " - + "local fetch: {}, remote fetch failure reported as local failure: {})", - srcNameTrimmed, srcAttemptIdentifier, srcAttemptIdentifier, connectFailed, - inputAttemptFetchFailure.isLocalFetch(), inputAttemptFetchFailure.isDiskErrorAtSource()); + LOG.info(srcNameTrimmed + ": " + "Fetch failed for src: " + srcAttemptIdentifier + + "InputIdentifier: " + srcAttemptIdentifier + ", connectFailed: " + + connectFailed); failedShufflesCounter.increment(1); inputContext.notifyProgress(); if (srcAttemptIdentifier == null) { @@ -813,9 +809,7 @@ public void fetchFailed(String host, srcAttemptIdentifier.getInputIdentifier(), srcAttemptIdentifier.getAttemptNumber()), srcAttemptIdentifier.getInputIdentifier(), - srcAttemptIdentifier.getAttemptNumber(), - inputAttemptFetchFailure.isLocalFetch(), - inputAttemptFetchFailure.isDiskErrorAtSource()); + srcAttemptIdentifier.getAttemptNumber()); List failedEvents = Lists.newArrayListWithCapacity(1); failedEvents.add(readError); diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java index 1f9d8f8c9d..1fcd621900 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/FetcherOrderedGrouped.java @@ -50,9 +50,7 @@ import org.apache.tez.runtime.library.common.sort.impl.TezIndexRecord; import org.apache.tez.runtime.library.common.sort.impl.TezSpillRecord; import org.apache.tez.runtime.library.exceptions.FetcherReadTimeoutException; -import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; -import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import com.google.common.annotations.VisibleForTesting; @@ -274,8 +272,7 @@ protected void copyFromHost(MapHost host) throws IOException { // On any error, faildTasks is not null and we exit // after putting back the remaining maps to the // yet_to_be_fetched list and marking the failed tasks. - InputAttemptFetchFailure[] failedTasks = null; - + InputAttemptIdentifier[] failedTasks = null; while (!remaining.isEmpty() && failedTasks == null) { InputAttemptIdentifier inputAttemptIdentifier = remaining.entrySet().iterator().next().getValue(); @@ -298,14 +295,25 @@ protected void copyFromHost(MapHost host) throws IOException { LOG.debug("Not reporting connection re-establishment failure since fetcher is stopped"); return; } - failedTasks = new InputAttemptFetchFailure[] { - new InputAttemptFetchFailure(getNextRemainingAttempt()) }; + failedTasks = new InputAttemptIdentifier[] {getNextRemainingAttempt()}; break; } } } - invokeCopyFailedForFailedTasks(host, failedTasks); + if (failedTasks != null && failedTasks.length > 0) { + if (stopped) { + if (LOG.isDebugEnabled()) { + LOG.debug("Ignoring copyMapOutput failures for tasks: " + Arrays.toString(failedTasks) + + " since Fetcher has been stopped"); + } + } else { + LOG.warn("copyMapOutput failed for tasks " + Arrays.toString(failedTasks)); + for (InputAttemptIdentifier left : failedTasks) { + scheduler.copyFailed(left, host, true, false, false); + } + } + } cleanupCurrentConnection(false); @@ -319,23 +327,6 @@ protected void copyFromHost(MapHost host) throws IOException { } } - private void invokeCopyFailedForFailedTasks(MapHost host, - InputAttemptFetchFailure[] failedTasks) { - if (failedTasks != null && failedTasks.length > 0) { - if (stopped) { - if (LOG.isDebugEnabled()) { - LOG.debug("Ignoring copyMapOutput failures for tasks: " + Arrays.toString(failedTasks) - + " since Fetcher has been stopped"); - } - } else { - LOG.warn("copyMapOutput failed for tasks " + Arrays.toString(failedTasks)); - for (InputAttemptFetchFailure left : failedTasks) { - scheduler.copyFailed(left, host, true, false); - } - } - } - } - @VisibleForTesting boolean setupConnection(MapHost host, Collection attempts) throws IOException { @@ -378,8 +369,7 @@ boolean setupConnection(MapHost host, Collection attempt for (InputAttemptIdentifier left : remaining.values()) { // Need to be handling temporary glitches .. // Report read error to the AM to trigger source failure heuristics - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(left).withCause(ie), host, connectSucceeded, - !connectSucceeded); + scheduler.copyFailed(left, host, connectSucceeded, !connectSucceeded, false); } return false; } @@ -403,8 +393,7 @@ protected void putBackRemainingMapOutputs(MapHost host) { } } - private static final InputAttemptFetchFailure[] EMPTY_ATTEMPT_ID_ARRAY = - new InputAttemptFetchFailure[0]; + private static InputAttemptIdentifier[] EMPTY_ATTEMPT_ID_ARRAY = new InputAttemptIdentifier[0]; private static class MapOutputStat { final InputAttemptIdentifier srcAttemptId; @@ -425,8 +414,8 @@ public String toString() { } } - protected InputAttemptFetchFailure[] copyMapOutput(MapHost host, DataInputStream input, - InputAttemptIdentifier inputAttemptIdentifier) throws FetcherReadTimeoutException { + protected InputAttemptIdentifier[] copyMapOutput(MapHost host, + DataInputStream input, InputAttemptIdentifier inputAttemptIdentifier) throws FetcherReadTimeoutException { MapOutput mapOutput = null; InputAttemptIdentifier srcAttemptId = null; long decompressedLength = 0; @@ -452,13 +441,7 @@ protected InputAttemptFetchFailure[] copyMapOutput(MapHost host, DataInputStream badIdErrs.increment(1); LOG.warn("Invalid map id: " + header.mapId + ", expected to start with " + InputAttemptIdentifier.PATH_PREFIX + ", partition: " + header.forReduce); - if (header.mapId.startsWith(ShuffleHandlerError.DISK_ERROR_EXCEPTION.toString())) { - //this should be treated as local fetch failure while reporting later - return new InputAttemptFetchFailure[] { - InputAttemptFetchFailure.fromDiskErrorAtSource(getNextRemainingAttempt()) }; - } - return new InputAttemptFetchFailure[] { - InputAttemptFetchFailure.fromAttempt(getNextRemainingAttempt()) }; + return new InputAttemptIdentifier[]{getNextRemainingAttempt()}; } else { LOG.debug("Already shutdown. Ignoring invalid map id error"); return EMPTY_ATTEMPT_ID_ARRAY; @@ -481,8 +464,7 @@ protected InputAttemptFetchFailure[] copyMapOutput(MapHost host, DataInputStream LOG.warn("Invalid map id ", e); // Don't know which one was bad, so consider this one bad and dont read // the remaining because we dont know where to start reading from. YARN-1773 - return new InputAttemptFetchFailure[] { - new InputAttemptFetchFailure(getNextRemainingAttempt()) }; + return new InputAttemptIdentifier[]{getNextRemainingAttempt()}; } else { if (LOG.isDebugEnabled()) { LOG.debug("Already shutdown. Ignoring invalid map id error. Exception: " + @@ -502,8 +484,7 @@ protected InputAttemptFetchFailure[] copyMapOutput(MapHost host, DataInputStream LOG.warn("Was expecting " + srcAttemptId + " but got null"); } assert (srcAttemptId != null); - return new InputAttemptFetchFailure[] { - new InputAttemptFetchFailure(getNextRemainingAttempt()) }; + return new InputAttemptIdentifier[]{srcAttemptId}; } else { LOG.debug("Already stopped. Ignoring verification failure."); return EMPTY_ATTEMPT_ID_ARRAY; @@ -597,10 +578,9 @@ protected InputAttemptFetchFailure[] copyMapOutput(MapHost host, DataInputStream srcAttemptId + " decomp: " + decompressedLength + ", " + compressedLength, ioe); if (srcAttemptId == null) { - return InputAttemptFetchFailure.fromAttempts(remaining.values()); + return remaining.values().toArray(new InputAttemptIdentifier[remaining.values().size()]); } else { - return new InputAttemptFetchFailure[] { - new InputAttemptFetchFailure(srcAttemptId) }; + return new InputAttemptIdentifier[]{srcAttemptId}; } } LOG.warn("Failed to shuffle output of " + srcAttemptId + @@ -608,8 +588,7 @@ protected InputAttemptFetchFailure[] copyMapOutput(MapHost host, DataInputStream // Inform the shuffle-scheduler mapOutput.abort(); - return new InputAttemptFetchFailure[] { - new InputAttemptFetchFailure(srcAttemptId) }; + return new InputAttemptIdentifier[] {srcAttemptId}; } return null; } @@ -738,8 +717,7 @@ protected void setupLocalDiskFetch(MapHost host) throws InterruptedException { if (!stopped) { hasFailures = true; ioErrs.increment(1); - scheduler.copyFailed(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttemptId).withCause(e), - host, true, false); + scheduler.copyFailed(srcAttemptId, host, true, false, true); LOG.warn("Failed to read local disk output of " + srcAttemptId + " from " + host.getHostIdentifier(), e); } else { diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleHeader.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleHeader.java index f074e897e8..9f883dbfdc 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleHeader.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleHeader.java @@ -103,11 +103,4 @@ public void write(DataOutput out) throws IOException { WritableUtils.writeVLong(out, uncompressedLength); WritableUtils.writeVInt(out, forReduce); } - - @Override - public String toString() { - return String.format( - "ShuffleHeader [mapId=%s, uncompressedLength=%d, compressedLength=%d, forReduce=%d]", mapId, - uncompressedLength, compressedLength, forReduce); - } } diff --git a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java index 8758caf830..736f41b6eb 100644 --- a/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java +++ b/tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/ShuffleScheduler.java @@ -80,7 +80,6 @@ import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils.FetchStatsLogger; import org.apache.tez.runtime.library.common.shuffle.HostPort; -import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.MapHost.HostPortPartition; import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.MapOutput.Type; @@ -178,7 +177,6 @@ enum ShuffleErrors { private final Referee referee; @VisibleForTesting final Map failureCounts = new HashMap(); - final Set uniqueHosts = Sets.newHashSet(); private final Map hostFailures = new HashMap(); private final InputContext inputContext; @@ -757,13 +755,16 @@ private void logProgress() { } } - public synchronized void copyFailed(InputAttemptFetchFailure fetchFailure, MapHost host, - boolean readError, boolean connectError) { + public synchronized void copyFailed(InputAttemptIdentifier srcAttempt, + MapHost host, + boolean readError, + boolean connectError, + boolean isLocalFetch) { failedShuffleCounter.increment(1); inputContext.notifyProgress(); - int failures = incrementAndGetFailureAttempt(fetchFailure.getInputAttemptIdentifier()); + int failures = incrementAndGetFailureAttempt(srcAttempt); - if (!fetchFailure.isLocalFetch()) { + if (!isLocalFetch) { /** * Track the number of failures that has happened since last completion. * This gets reset on a successful copy. @@ -788,11 +789,11 @@ public synchronized void copyFailed(InputAttemptFetchFailure fetchFailure, MapHo if (shouldInformAM) { //Inform AM. In case producer needs to be restarted, it is handled at AM. - informAM(fetchFailure); + informAM(srcAttempt); } //Restart consumer in case shuffle is not healthy - if (!isShuffleHealthy(fetchFailure)) { + if (!isShuffleHealthy(srcAttempt)) { return; } @@ -867,24 +868,21 @@ public void reportLocalError(IOException ioe) { } // Notify AM - private void informAM(InputAttemptFetchFailure fetchFailure) { - InputAttemptIdentifier srcAttempt = fetchFailure.getInputAttemptIdentifier(); + private void informAM(InputAttemptIdentifier srcAttempt) { LOG.info( - "{}: Reporting fetch failure for InputIdentifier: {} taskAttemptIdentifier: {}, " - + "local fetch: {}, remote fetch failure reported as local failure: {}) to AM.", - srcNameTrimmed, srcAttempt, - TezRuntimeUtils.getTaskAttemptIdentifier(inputContext.getSourceVertexName(), - srcAttempt.getInputIdentifier(), srcAttempt.getAttemptNumber()), - fetchFailure.isLocalFetch(), fetchFailure.isDiskErrorAtSource()); + srcNameTrimmed + ": " + "Reporting fetch failure for InputIdentifier: " + + srcAttempt + " taskAttemptIdentifier: " + TezRuntimeUtils + .getTaskAttemptIdentifier(inputContext.getSourceVertexName(), + srcAttempt.getInputIdentifier(), + srcAttempt.getAttemptNumber()) + " to AM."); List failedEvents = Lists.newArrayListWithCapacity(1); - failedEvents.add( - InputReadErrorEvent.create( - "Fetch failure for " - + TezRuntimeUtils.getTaskAttemptIdentifier(inputContext.getSourceVertexName(), - srcAttempt.getInputIdentifier(), srcAttempt.getAttemptNumber()) - + " to jobtracker.", - srcAttempt.getInputIdentifier(), srcAttempt.getAttemptNumber(), - fetchFailure.isLocalFetch(), fetchFailure.isDiskErrorAtSource())); + failedEvents.add(InputReadErrorEvent.create( + "Fetch failure for " + TezRuntimeUtils + .getTaskAttemptIdentifier(inputContext.getSourceVertexName(), + srcAttempt.getInputIdentifier(), + srcAttempt.getAttemptNumber()) + " to jobtracker.", + srcAttempt.getInputIdentifier(), + srcAttempt.getAttemptNumber())); inputContext.sendEvents(failedEvents); } @@ -1006,8 +1004,8 @@ private boolean isFetcherHealthy(String logContext) { return fetcherHealthy; } - boolean isShuffleHealthy(InputAttemptFetchFailure fetchFailure) { - InputAttemptIdentifier srcAttempt = fetchFailure.getInputAttemptIdentifier(); + boolean isShuffleHealthy(InputAttemptIdentifier srcAttempt) { + if (isAbortLimitExceeedFor(srcAttempt)) { return false; } @@ -1049,15 +1047,14 @@ boolean isShuffleHealthy(InputAttemptFetchFailure fetchFailure) { + ", pendingInputs=" + (numInputs - doneMaps) + ", fetcherHealthy=" + fetcherHealthy + ", reducerProgressedEnough=" + reducerProgressedEnough - + ", reducerStalled=" + reducerStalled - + ", hostFailures=" + hostFailures) + + ", reducerStalled=" + reducerStalled) + "]"; LOG.error(errorMsg); if (LOG.isDebugEnabled()) { LOG.debug("Host failures=" + hostFailures.keySet()); } // Shuffle knows how to deal with failures post shutdown via the onFailure hook - exceptionReporter.reportException(new IOException(errorMsg, fetchFailure.getCause())); + exceptionReporter.reportException(new IOException(errorMsg)); return false; } return true; diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestFetcher.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestFetcher.java index 05d4eb4145..db9c7afad0 100644 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestFetcher.java +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestFetcher.java @@ -32,7 +32,6 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; -import java.io.DataInputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -47,16 +46,10 @@ import org.apache.hadoop.fs.LocalFileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.yarn.api.records.ApplicationId; -import org.apache.tez.common.counters.TezCounters; import org.apache.tez.dag.api.TezConfiguration; -import org.apache.tez.runtime.api.InputContext; import org.apache.tez.runtime.library.api.TezRuntimeConfiguration; import org.apache.tez.runtime.library.common.InputAttemptIdentifier; -import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; -import org.apache.tez.runtime.library.common.shuffle.impl.ShuffleManager; -import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.ShuffleHeader; import org.apache.tez.runtime.library.common.sort.impl.TezIndexRecord; -import org.apache.tez.runtime.library.testutils.RuntimeTestUtils; import org.junit.Assert; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -86,8 +79,7 @@ public void testLocalFetchModeSetting() throws Exception { Fetcher fetcher = spy(builder.build()); FetchResult fr = new FetchResult(HOST, PORT, 0, 1, Arrays.asList(srcAttempts)); - Fetcher.HostFetchResult hfr = - new Fetcher.HostFetchResult(fr, InputAttemptFetchFailure.fromAttempts(srcAttempts), false); + Fetcher.HostFetchResult hfr = new Fetcher.HostFetchResult(fr, srcAttempts, false); doReturn(hfr).when(fetcher).setupLocalDiskFetch(); doReturn(null).when(fetcher).doHttpFetch(); doNothing().when(fetcher).shutdown(); @@ -159,7 +151,7 @@ public void testSetupLocalDiskFetch() throws Exception { }; final int FIRST_FAILED_ATTEMPT_IDX = 2; final int SECOND_FAILED_ATTEMPT_IDX = 4; - final int[] successfulAttempts = {0, 1, 3}; + final int[] sucessfulAttempts = {0, 1, 3}; TezConfiguration conf = new TezConfiguration(); conf.set(TezRuntimeConfiguration.TEZ_RUNTIME_OPTIMIZE_LOCAL_FETCH, "true"); @@ -214,24 +206,18 @@ public TezIndexRecord answer(InvocationOnMock invocation) throws Throwable { doNothing().when(fetcher).shutdown(); doNothing().when(callback).fetchSucceeded(anyString(), any(InputAttemptIdentifier.class), any(FetchedInput.class), anyLong(), anyLong(), anyLong()); - doNothing().when(callback).fetchFailed(anyString(), any(InputAttemptFetchFailure.class), eq(false)); + doNothing().when(callback).fetchFailed(anyString(), any(InputAttemptIdentifier.class), eq(false)); FetchResult fetchResult = fetcher.call(); verify(fetcher).setupLocalDiskFetch(); - // expect 3 successes and 2 failures - for (int i : successfulAttempts) { + // expect 3 sucesses and 2 failures + for (int i : sucessfulAttempts) { verifyFetchSucceeded(callback, srcAttempts[i], conf); } - verify(callback).fetchFailed(eq(HOST), - eq(InputAttemptFetchFailure - .fromCompositeAttemptLocalFetchFailure(srcAttempts[FIRST_FAILED_ATTEMPT_IDX])), - eq(false)); - verify(callback).fetchFailed(eq(HOST), - eq(InputAttemptFetchFailure - .fromCompositeAttemptLocalFetchFailure(srcAttempts[SECOND_FAILED_ATTEMPT_IDX])), - eq(false)); + verify(callback).fetchFailed(eq(HOST), eq(srcAttempts[FIRST_FAILED_ATTEMPT_IDX]), eq(false)); + verify(callback).fetchFailed(eq(HOST), eq(srcAttempts[SECOND_FAILED_ATTEMPT_IDX]), eq(false)); Assert.assertEquals("fetchResult host", fetchResult.getHost(), HOST); Assert.assertEquals("fetchResult partition", fetchResult.getPartition(), partition); @@ -318,30 +304,4 @@ public void testInputAttemptIdentifierMap() { Assert.assertTrue(expectedSrcAttempts[count++].toString().compareTo(key) == 0); } } - - @Test - public void testShuffleHandlerDiskErrorUnordered() - throws Exception { - Configuration conf = new Configuration(); - - InputContext inputContext = mock(InputContext.class); - doReturn(new TezCounters()).when(inputContext).getCounters(); - doReturn("vertex").when(inputContext).getSourceVertexName(); - - Fetcher.FetcherBuilder builder = new Fetcher.FetcherBuilder(mock(ShuffleManager.class), null, - null, ApplicationId.newInstance(0, 1), 1, null, "fetcherTest", conf, true, HOST, PORT, - false, true, false); - builder.assignWork(HOST, PORT, 0, 1, Arrays.asList(new InputAttemptIdentifier(0, 0))); - - Fetcher fetcher = builder.build(); - ShuffleHeader header = - new ShuffleHeader(ShuffleHandlerError.DISK_ERROR_EXCEPTION.toString(), -1, -1, -1); - DataInputStream input = RuntimeTestUtils.shuffleHeaderToDataInput(header); - - InputAttemptFetchFailure[] failures = - fetcher.fetchInputs(input, null, new InputAttemptIdentifier(0, 0)); - Assert.assertEquals(1, failures.length); - Assert.assertTrue(failures[0].isDiskErrorAtSource()); - Assert.assertFalse(failures[0].isLocalFetch()); - } } diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java index 4d84256a47..103f83d3cc 100644 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java @@ -52,6 +52,7 @@ import org.apache.tez.common.security.JobTokenIdentifier; import org.apache.tez.common.security.JobTokenSecretManager; import org.apache.tez.dag.api.TezConfiguration; +import org.apache.tez.dag.api.TezConstants; import org.apache.tez.runtime.api.Event; import org.apache.tez.runtime.api.ExecutionContext; import org.apache.tez.runtime.api.InputContext; @@ -61,9 +62,9 @@ import org.apache.tez.runtime.library.common.shuffle.FetchedInput; import org.apache.tez.runtime.library.common.shuffle.FetchedInputAllocator; import org.apache.tez.runtime.library.common.shuffle.Fetcher; -import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.FetchResult; import org.apache.tez.runtime.library.common.shuffle.InputHost; +import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; import org.apache.tez.runtime.library.shuffle.impl.ShuffleUserPayloads.DataMovementEventPayloadProto; import org.junit.After; import org.junit.Before; diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestFetcher.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestFetcher.java index 028fbce96a..5f7fe4ba7e 100644 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestFetcher.java +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestFetcher.java @@ -42,7 +42,6 @@ import java.net.URL; import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedList; @@ -73,10 +72,7 @@ import org.apache.tez.runtime.library.common.security.SecureShuffleUtils; import org.apache.tez.runtime.library.common.sort.impl.TezIndexRecord; import org.apache.tez.runtime.library.exceptions.FetcherReadTimeoutException; -import org.apache.tez.runtime.library.testutils.RuntimeTestUtils; -import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.apache.tez.runtime.library.common.shuffle.ShuffleUtils; -import org.apache.tez.runtime.library.common.shuffle.api.ShuffleHandlerError; import org.junit.Assert; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -245,7 +241,7 @@ public void testSetupLocalDiskFetch() throws Exception { ); final int FIRST_FAILED_ATTEMPT_IDX = 2; final int SECOND_FAILED_ATTEMPT_IDX = 4; - final int[] successfulAttemptsIndexes = { 0, 1, 3 }; + final int[] sucessfulAttemptsIndexes = { 0, 1, 3 }; doReturn(srcAttempts).when(scheduler).getMapsForHost(host); @@ -315,17 +311,13 @@ public TezIndexRecord answer(InvocationOnMock invocation) throws Throwable { spyFetcher.setupLocalDiskFetch(host); // should have exactly 3 success and 1 failure. - for (int i : successfulAttemptsIndexes) { + for (int i : sucessfulAttemptsIndexes) { for (int j = 0; j < host.getPartitionCount(); j++) { verifyCopySucceeded(scheduler, host, srcAttempts, i, j); } } - verify(scheduler).copyFailed( - eq(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(0))), - eq(host), eq(true), eq(false)); - verify(scheduler).copyFailed( - eq(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(0))), - eq(host), eq(true), eq(false)); + verify(scheduler).copyFailed(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(0), host, true, false, true); + verify(scheduler).copyFailed(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(0), host, true, false, true); verify(spyFetcher).putBackRemainingMapOutputs(host); verify(scheduler).putBackKnownMapOutput(host, srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX)); @@ -434,7 +426,7 @@ public void testSetupLocalDiskFetchAutoReduce() throws Exception { ); final int FIRST_FAILED_ATTEMPT_IDX = 2; final int SECOND_FAILED_ATTEMPT_IDX = 4; - final int[] successfulAttemptsIndexes = { 0, 1, 3 }; + final int[] sucessfulAttemptsIndexes = { 0, 1, 3 }; doReturn(srcAttempts).when(scheduler).getMapsForHost(host); final ConcurrentMap pathToIdentifierMap @@ -511,23 +503,15 @@ public TezIndexRecord answer(InvocationOnMock invocation) throws Throwable { spyFetcher.setupLocalDiskFetch(host); // should have exactly 3 success and 1 failure. - for (int i : successfulAttemptsIndexes) { + for (int i : sucessfulAttemptsIndexes) { for (int j = 0; j < host.getPartitionCount(); j++) { verifyCopySucceeded(scheduler, host, srcAttempts, i, j); } } - verify(scheduler).copyFailed( - eq(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(0))), - eq(host), eq(true), eq(false)); - verify(scheduler).copyFailed( - eq(InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(1))), - eq(host), eq(true), eq(false)); - verify(scheduler).copyFailed(eq( - InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(0))), - eq(host), eq(true), eq(false)); - verify(scheduler).copyFailed(eq( - InputAttemptFetchFailure.fromLocalFetchFailure(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(1))), - eq(host), eq(true), eq(false)); + verify(scheduler).copyFailed(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(0), host, true, false, true); + verify(scheduler).copyFailed(srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX).expand(1), host, true, false, true); + verify(scheduler).copyFailed(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(0), host, true, false, true); + verify(scheduler).copyFailed(srcAttempts.get(SECOND_FAILED_ATTEMPT_IDX).expand(1), host, true, false, true); verify(spyFetcher).putBackRemainingMapOutputs(host); verify(scheduler).putBackKnownMapOutput(host, srcAttempts.get(FIRST_FAILED_ATTEMPT_IDX)); @@ -646,8 +630,8 @@ public MapOutput answer(InvocationOnMock invocation) throws Throwable { //setup connection should be called twice (1 for connect and another for retry) verify(fetcher, times(2)).setupConnection(any(MapHost.class), any(Collection.class)); //since copyMapOutput consistently fails, it should call copyFailed once - verify(scheduler, times(1)).copyFailed(any(InputAttemptFetchFailure.class), any(MapHost.class), - anyBoolean(), anyBoolean()); + verify(scheduler, times(1)).copyFailed(any(InputAttemptIdentifier.class), any(MapHost.class), + anyBoolean(), anyBoolean(), anyBoolean()); verify(fetcher, times(1)).putBackRemainingMapOutputs(any(MapHost.class)); verify(scheduler, times(3)).putBackKnownMapOutput(any(MapHost.class), @@ -766,32 +750,6 @@ public void testInputAttemptIdentifierMap() { } } - @Test - public void testShuffleHandlerDiskErrorOrdered() - throws Exception { - MapHost mapHost = new MapHost(HOST, PORT, 0, 1); - InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(0, 0, "attempt"); - - FetcherOrderedGrouped fetcher = new FetcherOrderedGrouped(null, null, null, null, null, false, - 0, null, new TezConfiguration(), null, false, HOST, PORT, "src vertex", mapHost, - ioErrsCounter, wrongLengthErrsCounter, badIdErrsCounter, wrongMapErrsCounter, - connectionErrsCounter, wrongReduceErrsCounter, APP_ID, DAG_ID, false, false, true, false); - fetcher.remaining = new HashMap(); - - ShuffleHeader header = - new ShuffleHeader(ShuffleHandlerError.DISK_ERROR_EXCEPTION.toString(), -1, -1, -1); - DataInputStream input = RuntimeTestUtils.shuffleHeaderToDataInput(header); - - // copyMapOutput is used for remote fetch, this time it returns a fetch failure, which is fatal - // and should be treated as a local fetch failure - InputAttemptFetchFailure[] failures = - fetcher.copyMapOutput(mapHost, input, inputAttemptIdentifier); - - Assert.assertEquals(1, failures.length); - Assert.assertTrue(failures[0].isDiskErrorAtSource()); - Assert.assertFalse(failures[0].isLocalFetch()); - } - private RawLocalFileSystem getRawFs(Configuration conf) { try { return (RawLocalFileSystem) FileSystem.getLocal(conf).getRaw(); diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestShuffleScheduler.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestShuffleScheduler.java index b89ffb0ce9..fabfa270d7 100644 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestShuffleScheduler.java +++ b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/orderedgrouped/TestShuffleScheduler.java @@ -55,7 +55,6 @@ import org.apache.tez.runtime.library.api.TezRuntimeConfiguration; import org.apache.tez.runtime.library.common.CompositeInputAttemptIdentifier; import org.apache.tez.runtime.library.common.InputAttemptIdentifier; -import org.apache.tez.runtime.library.common.shuffle.InputAttemptFetchFailure; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -249,8 +248,8 @@ public void _testReducerHealth_1(Configuration conf) throws IOException { for (int i = 100; i < 199; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); } @@ -258,8 +257,9 @@ public void _testReducerHealth_1(Configuration conf) throws IOException { new InputAttemptIdentifier(200, 0, "attempt_"); //Should fail here and report exception as reducer is not healthy - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (200 % totalProducerNodes), 10000, 200, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (200 % + totalProducerNodes), + 10000, 200, 1), false, true, false); int minFailurePerHost = conf.getInt( TezRuntimeConfiguration.TEZ_RUNTIME_SHUFFLE_MIN_FAILURES_PER_HOST, @@ -330,8 +330,8 @@ public void testReducerHealth_2() throws IOException, InterruptedException { for (int i = 190; i < 200; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); } //Shuffle has not stalled. so no issues. @@ -342,8 +342,9 @@ public void testReducerHealth_2() throws IOException, InterruptedException { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(190, 0, "attempt_"); - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (190 % totalProducerNodes), 10000, 190, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + + (190 % totalProducerNodes), + 10000, 190, 1), false, true, false); //Even when it is stalled, need (320 - 300 = 20) * 3 = 60 failures verify(scheduler.reporter, times(0)).reportException(any(Throwable.class)); @@ -354,17 +355,16 @@ public void testReducerHealth_2() throws IOException, InterruptedException { for (int i = 190; i < 200; i++) { inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - InputAttemptFetchFailure failure = InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), - 10000, i, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); } assertEquals(61, scheduler.failedShufflesSinceLastCompletion); @@ -376,14 +376,12 @@ public void testReducerHealth_2() throws IOException, InterruptedException { for (int i = 110; i < 120; i++) { inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - InputAttemptFetchFailure failure = - InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); } // Should fail now due to fetcherHealthy. (stall has already happened and @@ -434,8 +432,8 @@ public void testReducerHealth_3() throws IOException { //1 fails (last fetch) InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(319, 0, "attempt_"); - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % totalProducerNodes), + 10000, 319, 1), false, true, false); //stall the shuffle scheduler.lastProgressTime = System.currentTimeMillis() - 1000000; @@ -443,13 +441,15 @@ public void testReducerHealth_3() throws IOException { assertEquals(scheduler.remainingMaps.get(), 1); //Retry for 3 more times - InputAttemptFetchFailure failure = InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); - scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 310, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 310, 1), - false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % + totalProducerNodes), + 10000, 319, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % + totalProducerNodes), + 10000, 310, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % + totalProducerNodes), + 10000, 310, 1), false, true, false); // failedShufflesSinceLastCompletion has crossed the limits. Throw error verify(shuffle, times(0)).reportException(any(Throwable.class)); @@ -487,15 +487,15 @@ public void testReducerHealth_4() throws IOException { for (int i = 0; i < 64; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - InputAttemptFetchFailure failure = - InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % + totalProducerNodes), 10000, i, 1), false, true, false); + + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % + totalProducerNodes), 10000, i, 1), false, true, false); + + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % + totalProducerNodes), 10000, i, 1), false, true, false); MapOutput mapOutput = MapOutput .createMemoryMapOutput(inputAttemptIdentifier, mock(FetchedInputAllocatorOrderedGrouped.class), @@ -518,8 +518,8 @@ public void testReducerHealth_4() throws IOException { //1 fails (last fetch) InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(319, 0, "attempt_"); - scheduler.copyFailed(new InputAttemptFetchFailure(inputAttemptIdentifier), - new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % totalProducerNodes), + 10000, 319, 1), false, true, false); //stall the shuffle (but within limits) scheduler.lastProgressTime = System.currentTimeMillis() - 100000; @@ -527,13 +527,15 @@ public void testReducerHealth_4() throws IOException { assertEquals(scheduler.remainingMaps.get(), 1); //Retry for 3 more times - InputAttemptFetchFailure failure = InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); - scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), - false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % + totalProducerNodes), + 10000, 319, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % + totalProducerNodes), + 10000, 319, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % + totalProducerNodes), + 10000, 319, 1), false, true, false); // failedShufflesSinceLastCompletion has crossed the limits. 20% of other nodes had failures as // well. However, it has failed only in one host. So this should proceed @@ -542,8 +544,9 @@ public void testReducerHealth_4() throws IOException { //stall the shuffle (but within limits) scheduler.lastProgressTime = System.currentTimeMillis() - 300000; - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (319 % totalProducerNodes), 10000, 319, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (319 % + totalProducerNodes), + 10000, 319, 1), false, true, false); verify(shuffle, times(1)).reportException(any(Throwable.class)); } @@ -589,9 +592,8 @@ public void testReducerHealth_5() throws IOException { //1 fails (last fetch) InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(318, 0, "attempt_"); - InputAttemptFetchFailure failure = new InputAttemptFetchFailure(inputAttemptIdentifier); - scheduler.copyFailed(failure, new MapHost("host" + (318 % totalProducerNodes), - 10000, 318, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (318 % totalProducerNodes), + 10000, 318, 1), false, true, false); //stall the shuffle scheduler.lastProgressTime = System.currentTimeMillis() - 1000000; @@ -599,12 +601,15 @@ public void testReducerHealth_5() throws IOException { assertEquals(scheduler.remainingMaps.get(), 1); //Retry for 3 more times - scheduler.copyFailed(failure, new MapHost("host" + (318 % totalProducerNodes), 10000, 318, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (318 % totalProducerNodes), 10000, 318, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (318 % totalProducerNodes), 10000, 318, 1), - false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (318 % + totalProducerNodes), + 10000, 318, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (318 % + totalProducerNodes), + 10000, 318, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (318 % + totalProducerNodes), + 10000, 318, 1), false, true, false); //Shuffle has not received the events completely. So do not bail out yet. verify(shuffle, times(0)).reportException(any(Throwable.class)); @@ -667,8 +672,8 @@ public void _testReducerHealth_6(Configuration conf) throws IOException { for (int i = 10; i < 15; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); } assertTrue(scheduler.failureCounts.size() >= 5); @@ -681,10 +686,10 @@ public void _testReducerHealth_6(Configuration conf) throws IOException { for (int i = 10; i < 15; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), - new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), false, true); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, new MapHost("host" + (i % totalProducerNodes), + 10000, i, 1), false, true, false); } boolean checkFailedFetchSinceLastCompletion = conf.getBoolean @@ -744,15 +749,18 @@ public void testReducerHealth_7() throws IOException { for (int i = 100; i < 199; i++) { InputAttemptIdentifier inputAttemptIdentifier = new InputAttemptIdentifier(i, 0, "attempt_"); - InputAttemptFetchFailure failure = InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); - scheduler.copyFailed(failure, new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), - false, true); + scheduler.copyFailed(inputAttemptIdentifier, + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true, false); + scheduler.copyFailed(inputAttemptIdentifier, + new MapHost("host" + (i % totalProducerNodes), 10000, i, 1), + false, true, false); } verify(shuffle, atLeast(1)).reportException(any(Throwable.class)); @@ -791,8 +799,7 @@ public void testPenalty() throws IOException, InterruptedException { MapHost mapHost = scheduler.pendingHosts.iterator().next(); //Fails to pull from host0. host0 should be added to penalties - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(inputAttemptIdentifier), mapHost, - false, true); + scheduler.copyFailed(inputAttemptIdentifier, mapHost, false, true, false); //Should not get host, as it is added to penalty loop MapHost host = scheduler.getHost(); @@ -986,8 +993,7 @@ public Void call() throws Exception { } for (int i = 0; i < 10; i++) { - scheduler.copyFailed(InputAttemptFetchFailure.fromAttempt(identifiers[0]), mapHosts[0], false, - false); + scheduler.copyFailed(identifiers[0], mapHosts[0], false, false, false); } ShuffleScheduler.Penalty[] penaltyArray = new ShuffleScheduler.Penalty[scheduler.getPenalties().size()]; scheduler.getPenalties().toArray(penaltyArray); diff --git a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/testutils/RuntimeTestUtils.java b/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/testutils/RuntimeTestUtils.java deleted file mode 100644 index 0885178ee5..0000000000 --- a/tez-runtime-library/src/test/java/org/apache/tez/runtime/library/testutils/RuntimeTestUtils.java +++ /dev/null @@ -1,44 +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.tez.runtime.library.testutils; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.DataInputStream; -import java.io.DataOutputStream; -import java.io.IOException; -import java.io.InputStream; - -import org.apache.tez.runtime.library.common.shuffle.orderedgrouped.ShuffleHeader; - -public final class RuntimeTestUtils { - - private RuntimeTestUtils() { - } - - public static DataInputStream shuffleHeaderToDataInput(ShuffleHeader header) throws IOException { - ByteArrayOutputStream byteOutput = new ByteArrayOutputStream(1000); - DataOutputStream output = new DataOutputStream(byteOutput); - header.write(output); - - InputStream inputStream = new ByteArrayInputStream(byteOutput.toByteArray()); - DataInputStream input = new DataInputStream(inputStream); - - return input; - } -} diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestInput.java b/tez-tests/src/test/java/org/apache/tez/test/TestInput.java index d6ce6ede7b..811ca3cc17 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestInput.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestInput.java @@ -263,7 +263,7 @@ public int doRead() { getContext().getUniqueIdentifier() + " index: " + index + " version: " + sourceInputVersion; LOG.info(msg); - events.add(InputReadErrorEvent.create(msg, index, sourceInputVersion, false, false)); + events.add(InputReadErrorEvent.create(msg, index, sourceInputVersion)); } } } diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java index 8a013e20ee..a25bbe99fe 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java @@ -113,6 +113,7 @@ public static Collection getParameters() { public static void setupDFSCluster() throws Exception { conf = new Configuration(); conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_EDITS_NOEDITLOGCHANNELFLUSH, false); + conf.setBoolean("fs.hdfs.impl.disable.cache", true); EditLogFileOutputStream.setShouldSkipFsyncForTesting(true); conf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR); miniDFSCluster = @@ -302,6 +303,7 @@ public static X509Certificate generateCertificate(String dn, KeyPair pair, int d certGen.addExtension(X509Extensions.SubjectAlternativeName, false, new GeneralNames(new GeneralName[] { new GeneralName(GeneralName.iPAddress, hostAddress), new GeneralName(GeneralName.dNSName, "localhost") })); + X500Principal dnName = new X500Principal(dn); certGen.setSerialNumber(sn); From 43fb152e23b081c55bd2620c41efdb9db90b6f28 Mon Sep 17 00:00:00 2001 From: Mark Bathori Date: Mon, 7 Feb 2022 22:36:07 +0100 Subject: [PATCH 7/9] Extend enabled protocols list --- .../src/test/java/org/apache/tez/test/TestSecureShuffle.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java index a25bbe99fe..eee33cfa07 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java @@ -139,6 +139,8 @@ public void setupTezCluster() throws Exception { setupKeyStores(); } conf.setBoolean(MRConfig.SHUFFLE_SSL_ENABLED_KEY, enableSSLInCluster); + //Hadoop 2.7 has only TLSv1 as enabled protocol which is not supported anymore so the protocol list needs to be extended + conf.set(SSLFactory.SSL_ENABLED_PROTOCOLS, "TLSv1,SSLv2Hello,TLSv1.1,TLSv1.2"); // 3 seconds should be good enough in local machine conf.setInt(TezRuntimeConfiguration.TEZ_RUNTIME_SHUFFLE_CONNECT_TIMEOUT, 3 * 1000); From 66ed3093885e56e5af122869a1b424ef2baacc02 Mon Sep 17 00:00:00 2001 From: Mark Bathori Date: Tue, 8 Feb 2022 11:36:13 +0100 Subject: [PATCH 8/9] Revert change This reverts commit b3a0e68d477bfc76c69c40dd3ad89a4800a7a8f7. --- .../src/test/java/org/apache/tez/test/TestSecureShuffle.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java index eee33cfa07..2768127ddd 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java @@ -113,7 +113,6 @@ public static Collection getParameters() { public static void setupDFSCluster() throws Exception { conf = new Configuration(); conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_EDITS_NOEDITLOGCHANNELFLUSH, false); - conf.setBoolean("fs.hdfs.impl.disable.cache", true); EditLogFileOutputStream.setShouldSkipFsyncForTesting(true); conf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR); miniDFSCluster = @@ -303,8 +302,7 @@ public static X509Certificate generateCertificate(String dn, KeyPair pair, int d String hostAddress = InetAddress.getLocalHost().getHostAddress(); certGen.addExtension(X509Extensions.SubjectAlternativeName, false, - new GeneralNames(new GeneralName[] { new GeneralName(GeneralName.iPAddress, hostAddress), - new GeneralName(GeneralName.dNSName, "localhost") })); + new GeneralNames(new GeneralName(GeneralName.iPAddress, hostAddress))); X500Principal dnName = new X500Principal(dn); From 219b72d5d4909cedb138865699f471c694b26770 Mon Sep 17 00:00:00 2001 From: Mark Bathori Date: Tue, 8 Feb 2022 18:01:31 +0100 Subject: [PATCH 9/9] Checkstyle fix --- .../src/test/java/org/apache/tez/test/TestSecureShuffle.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java index 2768127ddd..0261ff4b66 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestSecureShuffle.java @@ -138,7 +138,7 @@ public void setupTezCluster() throws Exception { setupKeyStores(); } conf.setBoolean(MRConfig.SHUFFLE_SSL_ENABLED_KEY, enableSSLInCluster); - //Hadoop 2.7 has only TLSv1 as enabled protocol which is not supported anymore so the protocol list needs to be extended + //Hadoop 2.7 has only TLSv1 as enabled protocol which is not supported anymore so the protocol list needs extension conf.set(SSLFactory.SSL_ENABLED_PROTOCOLS, "TLSv1,SSLv2Hello,TLSv1.1,TLSv1.2"); // 3 seconds should be good enough in local machine