[BEAM-9524] Fix for ib.show() executing indefinitely #11128
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rohdesamuel
force-pushed
the
streamingcache_refactor
branch
3 times, most recently
from
8f2efd8 to
68f2c6d
Compare
rohdesamuel
changed the title
rohdesamuel
force-pushed
the
streamingcache_refactor
branch
from
68f2c6d to
8cb62c1
Compare
Contributor
Author
|
R: @pabloem |
rohdesamuel
force-pushed
the
streamingcache_refactor
branch
from
8cb62c1 to
3e1d606
Compare
Member
|
retest this please |
pabloem
reviewed
Mar 18, 2020
sdks/python/apache_beam/runners/interactive/caching/streaming_cache.py
Outdated
Show resolved
Hide resolved
…estart Change-Id: I53aa32a75645086efffa091a53880a076c3a689d
Change-Id: I1ab6e7036172d7e2d07c774778a50e165df6bdca
rohdesamuel
force-pushed
the
streamingcache_refactor
branch
from
3e1d606 to
54a7e19
Compare
Member
|
retest this please |
1 similar comment
Member
|
retest this please |
davidyan74
reviewed
Mar 19, 2020
|
|
||
| @staticmethod | ||
| def from_str(r): | ||
| split = r.split('|') |
Contributor
There was a problem hiding this comment.
Check length of the list?
Contributor
There was a problem hiding this comment.
I think probably a better way is to do the following:
var, version, producer_version, pipeline_id = r.split('|')
return CacheKey(var, version, producer_version, pipeline_id)
Member
|
retest this please |
1 similar comment
Member
|
retest this please |
Change-Id: I247f37cd7acffb6ad796ce0fa8b54b0feff400d1
Member
|
retest this please |
2 similar comments
Member
|
retest this please |
Member
|
retest this please |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem was that the StreamingCache needed a way to communicate with the BackgroundCachingJob that it was done, so it knew when to stop tailing the file. This was previously done with capturing the user_pipeline when making the StreamingCache. However, the StreamingCache is used as a global cache for all pipelines. Thus, when the pipeline was redefined the StreamingCache had a stale reference to the previous pipeline and would loop forever.
The fix is to change the cache key of PCollection to be pipeline specific. In this PR, it can be seen that the pipeline id is used in the cache key. This is used to tell the StreamingCache what pipeline it's associated with. The StreamingCache can't be created with the user_pipeline because the pipeline_instrument object is dependent on the cache which creates the user pipeline.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.