hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-14971) Merge S3A committers into trunk
Date Fri, 27 Oct 2017 12:26:00 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-14971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16222267#comment-16222267
] 

ASF GitHub Bot commented on HADOOP-14971:
-----------------------------------------

Github user steveloughran commented on a diff in the pull request:

    https://github.com/apache/hadoop/pull/282#discussion_r147396821
  
    --- Diff: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Invoker.java
---
    @@ -0,0 +1,446 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.hadoop.fs.s3a;
    +
    +import java.io.IOException;
    +import java.io.InterruptedIOException;
    +import java.util.Optional;
    +
    +import com.amazonaws.AmazonClientException;
    +import com.amazonaws.SdkBaseException;
    +import com.google.common.base.Preconditions;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import org.apache.commons.lang3.StringUtils;
    +import org.apache.hadoop.io.retry.RetryPolicy;
    +
    +/**
    + * Class to provide lambda expression invocation of AWS operations.
    + *
    + * The core retry logic is in
    + * {@link #retryUntranslated(String, boolean, Retried, Operation)};
    + * the other {@code retry() and retryUntranslated()} calls are wrappers.
    + *
    + * The static {@link #once(String, String, Operation)} and
    + * {@link #once(String, String, VoidOperation)} calls take an operation and
    + * return it with AWS exceptions translated to IOEs of some form.
    + *
    + * The retry logic on a failure is defined by the retry policy passed in
    + * the constructor; the standard retry policy is {@link S3ARetryPolicy},
    + * though others may be used.
    + *
    + * The constructor also takes two {@link Retried} callbacks.
    + * The {@code caughtCallback} is called whenever an exception (IOE or AWS)
    + * is caught, before the retry processing looks at it.
    + * The {@code retryCallback} is invoked after a retry is scheduled
    + * but before the sleep.
    + * These callbacks can be used for reporting and incrementing statistics.
    + *
    + * The static {@link #quietly(String, String, VoidOperation)} and
    + * {@link #quietlyEval(String, String, Operation)} calls exist to take any
    + * operation and quietly catch & log at debug. The return value of
    + * {@link #quietlyEval(String, String, Operation)} is a java 8 optional,
    + * which can then be used in java8-expressions.
    + */
    +public class Invoker {
    +  private static final Logger LOG = LoggerFactory.getLogger(Invoker.class);
    +
    +  /**
    +   * Retry policy to use.
    +   */
    +  private final RetryPolicy retryPolicy;
    +
    +  /**
    +   * Default retry handler.
    +   */
    +  private final Retried retryCallback;
    +
    +  /**
    +   * Instantiate.
    +   * @param retryPolicy retry policy for all operations.
    +   * @param retryCallback standard retry policy
    +   */
    +  public Invoker(
    +      RetryPolicy retryPolicy,
    +      Retried retryCallback) {
    +    this.retryPolicy = retryPolicy;
    +    this.retryCallback = retryCallback;
    +  }
    +
    +  public RetryPolicy getRetryPolicy() {
    +    return retryPolicy;
    +  }
    +
    +  public Retried getRetryCallback() {
    +    return retryCallback;
    +  }
    +
    +  /**
    +   * Execute a function, translating any exception into an IOException.
    +   * @param action action to execute (used in error messages)
    +   * @param path path of work (used in error messages)
    +   * @param operation operation to execute
    +   * @param <T> type of return value
    +   * @return the result of the function call
    +   * @throws IOException any IOE raised, or translated exception
    +   */
    +  @Retries.OnceTranslated
    +  public static <T> T once(String action, String path, Operation<T> operation)
    +      throws IOException {
    +    try {
    +      return operation.execute();
    +    } catch (AmazonClientException e) {
    +      throw S3AUtils.translateException(action, path, e);
    +    }
    +  }
    +
    +  /**
    +   * Execute an operation with no result.
    +   * @param action action to execute (used in error messages)
    +   * @param path path of work (used in error messages)
    +   * @param operation operation to execute
    +   * @throws IOException any IOE raised, or translated exception
    +   */
    +  @Retries.OnceTranslated
    +  public static void once(String action, String path, VoidOperation operation)
    +      throws IOException {
    +    once(action, path,
    +        () -> {
    +          operation.execute();
    +          return null;
    +        });
    +  }
    +
    +  /**
    +   * Execute a void operation with retry processing.
    +   * @param action action to execute (used in error messages)
    +   * @param path path of work (used in error messages)
    +   * @param idempotent does the operation have semantics
    +   * which mean that it can be retried even if was already executed?
    +   * @param retrying callback on retries
    +   * @param operation operation to execute
    +   * @throws IOException any IOE raised, or translated exception
    +   */
    +  @Retries.RetryTranslated
    +  public void retry(String action,
    +      String path,
    +      boolean idempotent,
    +      Retried retrying,
    +      VoidOperation operation)
    +      throws IOException {
    +    retry(action, path, idempotent, retrying,
    +        () -> {
    +          operation.execute();
    +          return null;
    +        }
    +    );
    +  }
    +
    +  /**
    +   * Execute a void operation with  the default retry callback invoked.
    +   * @param action action to execute (used in error messages)
    +   * @param path path of work (used in error messages)
    +   * @param idempotent does the operation have semantics
    +   * which mean that it can be retried even if was already executed?
    +   * @param operation operation to execute
    +   * @throws IOException any IOE raised, or translated exception
    +   */
    +  @Retries.RetryTranslated
    +  public void retry(String action,
    +      String path,
    +      boolean idempotent,
    +      VoidOperation operation)
    +      throws IOException {
    +    retry(action, path, idempotent, retryCallback, operation);
    +  }
    +
    +  /**
    +   * Execute a function with the default retry callback invoked.
    +   * @param action action to execute (used in error messages)
    +   * @param path path of work (used in error messages)
    +   * @param idempotent does the operation have semantics
    +   * which mean that it can be retried even if was already executed?
    +   * @param operation operation to execute
    +   * @param <T> type of return value
    +   * @return the result of the call
    +   * @throws IOException any IOE raised, or translated exception
    +   */
    +  @Retries.RetryTranslated
    +  public <T> T retry(String action,
    +      String path,
    +      boolean idempotent,
    +      Operation<T> operation)
    +      throws IOException {
    +
    +    return retry(action, path, idempotent, retryCallback, operation);
    +  }
    +
    +  /**
    +   * Execute a function with retry processing.
    +   * Uses {@link #once(String, String, Operation)} as the inner
    +   * invocation mechanism before retry logic is performed.
    +   * @param <T> type of return value
    +   * @param action action to execute (used in error messages)
    +   * @param path path of work (used in error messages)
    +   * @param idempotent does the operation have semantics
    +   * which mean that it can be retried even if was already executed?
    +   * @param retrying callback on retries
    +   * @param operation operation to execute
    +   * @return the result of the call
    +   * @throws IOException any IOE raised, or translated exception
    +   */
    +  @Retries.RetryTranslated
    +  public <T> T retry(
    +      String action,
    +      String path,
    +      boolean idempotent,
    +      Retried retrying,
    +      Operation<T> operation)
    +      throws IOException {
    +    return retryUntranslated(
    +        toDescription(action, path),
    +        idempotent,
    +        retrying,
    +        () -> once(action, path, operation));
    +  }
    +
    +  /**
    +   * Execute a function with retry processing and no translation.
    +   * and the default retry callback.
    +   * @param text description for the catching callback
    +   * @param idempotent does the operation have semantics
    +   * which mean that it can be retried even if was already executed?
    +   * @param operation operation to execute
    +   * @param <T> type of return value
    +   * @return the result of the call
    +   * @throws IOException any IOE raised
    +   * @throws RuntimeException any Runtime exception raised
    +   */
    +  @Retries.RetryRaw
    +  public <T> T retryUntranslated(
    +      String text,
    +      boolean idempotent,
    +      Operation<T> operation) throws IOException {
    +    return retryUntranslated(text, idempotent,
    +        retryCallback, operation);
    +  }
    +
    +  /**
    +   * Execute a function with retry processing: AWS SDK Exceptions
    +   * are <i>not</i> translated.
    +   * This is method which the others eventually invoke.
    +   * @param <T> type of return value
    +   * @param text text to include in messages
    +   * @param idempotent does the operation have semantics
    +   * which mean that it can be retried even if was already executed?
    +   * @param retrying callback on retries
    +   * @param operation operation to execute
    +   * @return the result of the call
    +   * @throws IOException any IOE raised
    +   * @throws SdkBaseException any AWS exception raised
    +   */
    +  @Retries.RetryRaw
    +  public <T> T retryUntranslated(
    +      String text,
    +      boolean idempotent,
    +      Retried retrying,
    +      Operation<T> operation) throws IOException {
    +
    +    Preconditions.checkArgument(retrying != null, "null retrying argument");
    +    int retryCount = 0;
    +    Exception caught;
    +    RetryPolicy.RetryAction retryAction;
    +    boolean shouldRetry;
    +    do {
    +      try {
    +        // execute the operation, returning if successful
    +        return operation.execute();
    +      } catch (IOException | SdkBaseException e) {
    +        caught = e;
    +      }
    --- End diff --
    
    I'm taking the view that anything which isn't some network IOE or from the AWS SDK is
not something to go near. Invalid args, NPE, assertions Interruption, OOM etc, all stuff there's
no attempt to retry. Bear in mind the logic for the S3ARetryPolicy ignores all of those anyway,
so even if caught, they'd not be retried. (the S3Guard dataAccessPolicy is exponential backoff,
as before, but as its no longer catching NPEs &c it is having it scope of what-to-retry
loosened slightly. 
    
    Oh, and by only handling IOEs we can keep the signature of the invoke calls always {{throws
IOE}}.
    
    If you want a non IOE, non SDK exception to be retried on your lambda-exp: catch and translate
before you invoke()


> Merge S3A committers into trunk
> -------------------------------
>
>                 Key: HADOOP-14971
>                 URL: https://issues.apache.org/jira/browse/HADOOP-14971
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3
>    Affects Versions: 3.0.0
>            Reporter: Steve Loughran
>            Assignee: Steve Loughran
>
> Merge the HADOOP-13786 committer into trunk. This branch is being set up as a github
PR for review there & to keep it out the mailboxes of the watchers on the main JIRA



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Mime
View raw message