[BEAM-1937] Free PTransform Names if they are being Replaced#2506
[BEAM-1937] Free PTransform Names if they are being Replaced#2506tgroh wants to merge 2 commits intoapache:masterfrom
Conversation
|
R: @dhalperi |
|
Refer to this link for build results (access rights to CI server needed): |
|
Build doesn't pass. |
Naming is based on what's in the graph, not what once was there.
tgroh
force-pushed
the
replacement_names_freed
branch
from
9fbe5e3 to
530ef12
Compare
|
Should be fixed |
|
Refer to this link for build results (access rights to CI server needed): Build result: ABORTED[...truncated 2.80 MB...]2017-04-12T19:28:33.313 [INFO] Apache Beam :: SDKs :: Java :: Java 8 Tests ........ SUCCESS [ 17.173 s]2017-04-12T19:28:33.314 [INFO] Apache Beam :: SDKs :: Python ...................... SUCCESS [15:57 min]2017-04-12T19:28:33.314 [INFO] Apache Beam :: Runners :: Flink .................... SUCCESS [ 49.882 s]2017-04-12T19:28:33.314 [INFO] Apache Beam :: Runners :: Flink :: Core ............ SUCCESS [02:24 min]2017-04-12T19:28:33.314 [INFO] Apache Beam :: Runners :: Flink :: Examples ........ SUCCESS [01:00 min]2017-04-12T19:28:33.314 [INFO] Apache Beam :: Runners :: Apex ..................... SUCCESS [03:06 min]2017-04-12T19:28:33.314 [INFO] Apache Beam :: Examples ............................ SUCCESS [01:23 min]2017-04-12T19:28:33.314 [INFO] Apache Beam :: Examples :: Java .................... SUCCESS [25:43 min]2017-04-12T19:28:33.314 [INFO] Apache Beam :: Examples :: Java 8 .................. SUCCESS [ 27.139 s]2017-04-12T19:28:33.314 [INFO] Apache Beam :: SDKs :: Java :: Aggregated Javadoc .. SUCCESS [ 42.131 s]2017-04-12T19:28:33.314 [INFO] ------------------------------------------------------------------------2017-04-12T19:28:33.314 [INFO] BUILD SUCCESS2017-04-12T19:28:33.314 [INFO] ------------------------------------------------------------------------2017-04-12T19:28:33.314 [INFO] Total time: 01:36 h2017-04-12T19:28:33.314 [INFO] Finished at: 2017-04-12T19:28:33+00:002017-04-12T19:28:33.960 [INFO] Final Memory: 271M/1599M2017-04-12T19:28:33.960 [INFO] ------------------------------------------------------------------------Waiting for Jenkins to finish collecting dataBuild timed out (after 100 minutes). Marking the build as aborted.AbortedBuild was abortedchannel stoppedSetting status of 530ef12 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/9455/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install--none-- |
|
retest this please |
|
Build is successful, but got killed because it took too long to start |
|
Refer to this link for build results (access rights to CI server needed): |
| // This node will be replaced. It should not be visited. | ||
| return CompositeBehavior.DO_NOT_ENTER_TRANSFORM; | ||
| clearingNamesFor = node; | ||
| freedNames.add(node.getFullName()); |
There was a problem hiding this comment.
It seems simpler and clearer to me to preserve the existing behavior (don't enter child).
Instead, add another function that is "collect all names to be replaces" that does an inner recursive traverse of the node to be replaced.
Traverses that do two things in modes based on local variables being null or not is ... confusing.
There was a problem hiding this comment.
Right now there's no way to traverse an arbitrary subgraph of a TransformHierarchy or Pipeline;
Instead I've changed this to track if the parent node is to-be-freed, starting with any replacement node. I think it's significantly more readable than the nullable variable, but still requires continuing to traverse the graph after we've matched a node to replace. Alternatively I could split this into two traversals. WDYT?
|
Refer to this link for build results (access rights to CI server needed): |
4 participants
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
Naming is based on what's in the graph, not what once was there.