You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/07/01 13:48:34 UTC

[GitHub] [flink] zhuyufeng0809 opened a new pull request, #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

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

   ## What is the purpose of the change
   
   Fix a never reached code in ProcessMemoryUtils Class
   
   
   ## Brief change log
   
   Change the location of sanityCheckTotalProcessMemory method call, make sure when explicitly set taskmanager.memory.process.size, check the derived memory and the configured memory are equal
   
   ## Verifying this change
   
   This change is already covered by existing tests, i ran TaskExecutorProcessUtilsTest Class and JobManagerProcessUtilsTest Class locally, they both passed the test
   
   ## 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] xintongsong closed pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
xintongsong closed pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class
URL: https://github.com/apache/flink/pull/19991


-- 
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] zhuyufeng0809 commented on pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
zhuyufeng0809 commented on PR #19991:
URL: https://github.com/apache/flink/pull/19991#issuecomment-1157539639

   Hi @Myasuka , could you help me to review this PR


-- 
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 #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19991:
URL: https://github.com/apache/flink/pull/19991#issuecomment-1157521528

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "bc402b22393ba2698f4d6824f87f8454aeb74337",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "bc402b22393ba2698f4d6824f87f8454aeb74337",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * bc402b22393ba2698f4d6824f87f8454aeb74337 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] Myasuka commented on pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
Myasuka commented on PR #19991:
URL: https://github.com/apache/flink/pull/19991#issuecomment-1172321518

   @zhuyufeng0809 Why you closed this PR? I think you can ping related committer if the PR moves no progress.


-- 
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] zhuyufeng0809 commented on pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
zhuyufeng0809 commented on PR #19991:
URL: https://github.com/apache/flink/pull/19991#issuecomment-1172369454

   I click the restore button, which seems to restore the branch I deleted


-- 
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] xintongsong commented on pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
xintongsong commented on PR #19991:
URL: https://github.com/apache/flink/pull/19991#issuecomment-1173282190

   I tend not to merge this change.
   
   First of all, the sanity check is unnecessary for both the `if-` and the `else-` branches here. For the `if-` branch, the sanity is guaranteed by the logics in `deriveJvmOverheadFromTotalFlinkMemoryAndOtherComponents`. So if we want to improve these codes, we probably should simply remove the sanity check.
   
   More importantly, I don't see how this change benefits Flink. The "problem" it tries to fix here does not have any actual damage. Admittedly, the code base of Flink is not perfect. Yet, we don't encourage pure cleanup changes like this.


-- 
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] zhuyufeng0809 commented on pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
zhuyufeng0809 commented on PR #19991:
URL: https://github.com/apache/flink/pull/19991#issuecomment-1172853473

   Hi @xintongsong , could you help me to review this PR


-- 
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] zhuyufeng0809 commented on pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
zhuyufeng0809 commented on PR #19991:
URL: https://github.com/apache/flink/pull/19991#issuecomment-1173286177

   OK, thank you for your Review


-- 
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] zhuyufeng0809 closed pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class

Posted by GitBox <gi...@apache.org>.
zhuyufeng0809 closed pull request #19991: [FLINK-28065][Runtime/Configuration] Fix a never reached code in ProcessMemoryUtils Class
URL: https://github.com/apache/flink/pull/19991


-- 
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