You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "WencongLiu (via GitHub)" <gi...@apache.org> on 2023/03/30 10:47:19 UTC

[GitHub] [flink] WencongLiu opened a new pull request, #22307: [FLINK-31643] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle

WencongLiu opened a new pull request, #22307:
URL: https://github.com/apache/flink/pull/22307

   
   ## What is the purpose of the change
   
   *Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle.*
   
   
   ## Brief change log
   
     - *Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle.*
   
   
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
     - The serializers: (no)
     - The runtime per-record code paths (performance sensitive): (no)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
     - The S3 file system connector: (no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (no)
   


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] WencongLiu commented on pull request #22307: [FLINK-31643][core] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle

Posted by "WencongLiu (via GitHub)" <gi...@apache.org>.
WencongLiu commented on PR #22307:
URL: https://github.com/apache/flink/pull/22307#issuecomment-1491214003

   @flinkbot run azure


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] reswqa commented on a diff in pull request #22307: [FLINK-31643][core] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on code in PR #22307:
URL: https://github.com/apache/flink/pull/22307#discussion_r1153157914


##########
flink-core/src/main/java/org/apache/flink/configuration/NettyShuffleEnvironmentOptions.java:
##########
@@ -379,6 +380,17 @@ public class NettyShuffleEnvironmentOptions {
                                     + "tasks have occupied all the buffers and the downstream tasks are waiting for the exclusive buffers. The timeout breaks"
                                     + "the tie by failing the request of exclusive buffers and ask users to increase the number of total buffers.");
 
+    @Documentation.Section(Documentation.Sections.ALL_TASK_MANAGER_NETWORK)
+    @Experimental
+    @Internal
+    public static final ConfigOption<Boolean> NETWORK_HYBRID_SHUFFLE_ENABLE_TIERED_STORE =
+            ConfigOptions.key("taskmanager.network.hybrid-shuffle.enable-tiered-store")
+                    .booleanType()
+                    .defaultValue(false)
+                    .withDescription(
+                            "This is a temporary configuration to enable tiered store architecture for hybrid shuffle mode. When the new architecture reaches "

Review Comment:
   TBH, I'm not sure whether we should imply that this option will be removed for now. After all we may go through the following three process to remove this:
   
   1. Manually set this option to `true`.
   2. Set the default value to `true`.
   3. Remove the old hybrid shuffle implementation and this option.
   
   



##########
flink-core/src/main/java/org/apache/flink/configuration/NettyShuffleEnvironmentOptions.java:
##########
@@ -379,6 +380,17 @@ public class NettyShuffleEnvironmentOptions {
                                     + "tasks have occupied all the buffers and the downstream tasks are waiting for the exclusive buffers. The timeout breaks"
                                     + "the tie by failing the request of exclusive buffers and ask users to increase the number of total buffers.");
 
+    @Documentation.Section(Documentation.Sections.ALL_TASK_MANAGER_NETWORK)

Review Comment:
   IIUC, this option will not expose to user before this efforts all finished.
   We'd better add `@Documentation.ExcludeFromDocumentation` to exclude this option from document.
   Maybe we should also add `WIP` prefix to the key to remind user do not use this currently.



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] WencongLiu commented on a diff in pull request #22307: [FLINK-31643][core] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle

Posted by "WencongLiu (via GitHub)" <gi...@apache.org>.
WencongLiu commented on code in PR #22307:
URL: https://github.com/apache/flink/pull/22307#discussion_r1153198093


##########
flink-core/src/main/java/org/apache/flink/configuration/NettyShuffleEnvironmentOptions.java:
##########
@@ -379,6 +380,17 @@ public class NettyShuffleEnvironmentOptions {
                                     + "tasks have occupied all the buffers and the downstream tasks are waiting for the exclusive buffers. The timeout breaks"
                                     + "the tie by failing the request of exclusive buffers and ask users to increase the number of total buffers.");
 
+    @Documentation.Section(Documentation.Sections.ALL_TASK_MANAGER_NETWORK)

Review Comment:
   Done. Currently this option will not be presented in the document, which is verified by "mvn -Pgenerate-config-docs install -Dfast -DskipTests".



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] WencongLiu closed pull request #22307: [FLINK-31643][core] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle

Posted by "WencongLiu (via GitHub)" <gi...@apache.org>.
WencongLiu closed pull request #22307: [FLINK-31643][core] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle
URL: https://github.com/apache/flink/pull/22307


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] WencongLiu closed pull request #22307: [FLINK-31643] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle

Posted by "WencongLiu (via GitHub)" <gi...@apache.org>.
WencongLiu closed pull request #22307: [FLINK-31643] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle
URL: https://github.com/apache/flink/pull/22307


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] flinkbot commented on pull request #22307: [FLINK-30643] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22307:
URL: https://github.com/apache/flink/pull/22307#issuecomment-1490085280

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "76e60ce83d20cb317c097ed10480ac8f89de7e65",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "76e60ce83d20cb317c097ed10480ac8f89de7e65",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 76e60ce83d20cb317c097ed10480ac8f89de7e65 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] reswqa commented on a diff in pull request #22307: [FLINK-31643][core] Introduce a temporary configuration to enable the tiered store architecture for hybrid shuffle

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on code in PR #22307:
URL: https://github.com/apache/flink/pull/22307#discussion_r1153157914


##########
flink-core/src/main/java/org/apache/flink/configuration/NettyShuffleEnvironmentOptions.java:
##########
@@ -379,6 +380,17 @@ public class NettyShuffleEnvironmentOptions {
                                     + "tasks have occupied all the buffers and the downstream tasks are waiting for the exclusive buffers. The timeout breaks"
                                     + "the tie by failing the request of exclusive buffers and ask users to increase the number of total buffers.");
 
+    @Documentation.Section(Documentation.Sections.ALL_TASK_MANAGER_NETWORK)
+    @Experimental
+    @Internal
+    public static final ConfigOption<Boolean> NETWORK_HYBRID_SHUFFLE_ENABLE_TIERED_STORE =
+            ConfigOptions.key("taskmanager.network.hybrid-shuffle.enable-tiered-store")
+                    .booleanType()
+                    .defaultValue(false)
+                    .withDescription(
+                            "This is a temporary configuration to enable tiered store architecture for hybrid shuffle mode. When the new architecture reaches "

Review Comment:
   TBH, I'm not sure whether we should imply that this option will be removed for now. After all we may go through the following three process lates server versions to remove this:
   
   1. Manually set this option to `true`.
   2. Set the default value to `true`.
   3. Remove the old hybrid shuffle implementation and this option.
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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