You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/03/02 18:52:42 UTC

[GitHub] [beam] dpcollins-google opened a new pull request #16992: Make ScopedReadStateSupplier final

dpcollins-google opened a new pull request #16992:
URL: https://github.com/apache/beam/pull/16992


   This is so it does not bind StreamingModeExecutionContext which has high memory cost
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] scwhittle commented on pull request #16992: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
scwhittle commented on pull request #16992:
URL: https://github.com/apache/beam/pull/16992#issuecomment-1066975248


   error looked unrelated to me
   


-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] aaltay commented on pull request #16992: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #16992:
URL: https://github.com/apache/beam/pull/16992#issuecomment-1064337351


   @dpcollins-google - Is the test failure related to your change?


-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] aaltay commented on pull request #16992: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #16992:
URL: https://github.com/apache/beam/pull/16992#issuecomment-1068300577






-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] scwhittle commented on pull request #16992: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
scwhittle commented on pull request #16992:
URL: https://github.com/apache/beam/pull/16992#issuecomment-1066975065


   Run Java PreCommit


-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] TheNeuralBit merged pull request #16992: Minor: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
TheNeuralBit merged pull request #16992:
URL: https://github.com/apache/beam/pull/16992


   


-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] scwhittle commented on a change in pull request #16992: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
scwhittle commented on a change in pull request #16992:
URL: https://github.com/apache/beam/pull/16992#discussion_r818010542



##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingModeExecutionContext.java
##########
@@ -481,6 +481,25 @@ String getStateFamily(NameContext nameContext) {
     return nameContext.userName() == null ? null : stateNameMap.get(nameContext.userName());
   }
 
+  private static class ScopedReadStateSupplier implements Supplier<Closeable> {
+    private final ExecutionState readState;
+    private final @Nullable ExecutionStateTracker stateTracker;
+
+    ScopedReadStateSupplier(
+        DataflowOperationContext operationContext, ExecutionStateTracker stateTracker) {
+      this.readState = operationContext.newExecutionState("windmill-read");

Review comment:
       nit: only do this if stateTracker is non-null?




-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] dpcollins-google commented on a change in pull request #16992: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
dpcollins-google commented on a change in pull request #16992:
URL: https://github.com/apache/beam/pull/16992#discussion_r818022213



##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingModeExecutionContext.java
##########
@@ -481,6 +481,25 @@ String getStateFamily(NameContext nameContext) {
     return nameContext.userName() == null ? null : stateNameMap.get(nameContext.userName());
   }
 
+  private static class ScopedReadStateSupplier implements Supplier<Closeable> {
+    private final ExecutionState readState;
+    private final @Nullable ExecutionStateTracker stateTracker;
+
+    ScopedReadStateSupplier(
+        DataflowOperationContext operationContext, ExecutionStateTracker stateTracker) {
+      this.readState = operationContext.newExecutionState("windmill-read");

Review comment:
       Nack- this would be a change in the code, I'm not sure that the fact of calling newExecutionState but never committing is a no-op.




-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] dpcollins-google commented on a change in pull request #16992: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
dpcollins-google commented on a change in pull request #16992:
URL: https://github.com/apache/beam/pull/16992#discussion_r818022213



##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingModeExecutionContext.java
##########
@@ -481,6 +481,25 @@ String getStateFamily(NameContext nameContext) {
     return nameContext.userName() == null ? null : stateNameMap.get(nameContext.userName());
   }
 
+  private static class ScopedReadStateSupplier implements Supplier<Closeable> {
+    private final ExecutionState readState;
+    private final @Nullable ExecutionStateTracker stateTracker;
+
+    ScopedReadStateSupplier(
+        DataflowOperationContext operationContext, ExecutionStateTracker stateTracker) {
+      this.readState = operationContext.newExecutionState("windmill-read");

Review comment:
       Nack- this would be a change in the code, I'm not sure that the fact of calling newExecutionState but never entering is a no-op.




-- 
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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] dpcollins-google commented on pull request #16992: Make ScopedReadStateSupplier final

Posted by GitBox <gi...@apache.org>.
dpcollins-google commented on pull request #16992:
URL: https://github.com/apache/beam/pull/16992#issuecomment-1069196068


   Run Java PreCommit


-- 
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: github-unsubscribe@beam.apache.org

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