[GitHub] [spark] Victsm commented on a change in pull request #33340: [SPARK-32915][SHUFFLE][FOLLOW-UP] Rename classes in shuffle RPC used for block push operations

[GitBox <gi...@apache.org>]

Victsm commented on a change in pull request #33340:
URL: https://github.com/apache/spark/pull/33340#discussion_r675230242



##########
File path: common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RetryingBlockTransferor.java
##########
@@ -34,44 +34,48 @@
 import org.apache.spark.network.util.TransportConf;
 
 /**
- * Wraps another BlockFetcher with the ability to automatically retry fetches which fail due to
- * IOExceptions, which we hope are due to transient network conditions.
+ * Wraps another BlockFetcher or BlockPusher with the ability to automatically retry fetches or
+ * pushes which fail due to IOExceptions, which we hope are due to transient network conditions.
  *
- * This fetcher provides stronger guarantees regarding the parent BlockFetchingListener. In
+ * This transferor provides stronger guarantees regarding the parent BlockTransferListener. In
  * particular, the listener will be invoked exactly once per blockId, with a success or failure.
  */
-public class RetryingBlockFetcher {
+public class RetryingBlockTransferor {
 
   /**
-   * Used to initiate the first fetch for all blocks, and subsequently for retrying the fetch on any
-   * remaining blocks.
+   * Used to initiate the first fetch or push for all blocks, and subsequently for retrying the
+   * fetch or push on any remaining blocks.
    */
-  public interface BlockFetchStarter {
+  public interface BlockTransferStarter {
     /**
-     * Creates a new BlockFetcher to fetch the given block ids which may do some synchronous
-     * bootstrapping followed by fully asynchronous block fetching.
-     * The BlockFetcher must eventually invoke the Listener on every input blockId, or else this
-     * method must throw an exception.
+     * Creates a new BlockFetcher or BlockPusher to fetch or push the given block ids which may do
+     * some synchronous bootstrapping followed by fully asynchronous block transferring.
+     * The BlockFetcher or BlockPusher must eventually invoke the Listener on every input blockId,
+     * or else this method must throw an exception.
      *
      * This method should always attempt to get a new TransportClient from the
      * {@link org.apache.spark.network.client.TransportClientFactory} in order to fix connection
      * issues.
      */
-    void createAndStart(String[] blockIds, BlockFetchingListener listener)
+    void createAndStart(String[] blockIds, BlockTransferListener listener)
          throws IOException, InterruptedException;
   }
 
   /** Shared executor service used for waiting and retrying. */
   private static final ExecutorService executorService = Executors.newCachedThreadPool(
-    NettyUtils.createThreadFactory("Block Fetch Retry"));
+    NettyUtils.createThreadFactory("Block Transfer Retry"));

Review comment:
       Since this is a static final variable, would have to keep its naming generic.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org