Skip to content

Conversation

@kennknowles
Copy link
Member

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

This will make way for using the evaluator in contexts where cloning
is not appropriate, such as the evaluator for GroupAlsoByWindow

This will make way for using the evluator in contexts where cloning
is not appropriate, such as evaluator GroupAlsoByWindow
@kennknowles
Copy link
Member Author

R: @tgroh for in-process runner
R: @lukecwik for committer

@tgroh
Copy link
Member

tgroh commented Apr 29, 2016

LGTM

@tgroh
Copy link
Member

tgroh commented Apr 29, 2016

Actually, upon second thought;

Given that we are expected to reuse DoFns, this could fold in another change, and provide the ParDo to the evaluator as a ThreadLocal. Doing so gives a similar behavior (using ThreadLocal#initialValue()) instead of supplier.get(). That should get the same benefit (given that we only ever call supplier.get() once), but also enforce the requirement for DoFns to be used in a single thread, plus reuse.

@kennknowles
Copy link
Member Author

I believe the decision of how and when to re-use a DoFn should occur outside the evaluator, but I could be mistaken. Not totally sure how that jives with your comment.

@kennknowles
Copy link
Member Author

Via @lukecwik: instead, this logic should move even further out. A new evaluator should be created whenever a DoFn would be cloned anyhow, so there's no need to abstract this into a supplier, but just move the cloning out to the ParDoMultiEvaluatorFactory, etc. I will combine that smaller change directly into my GBK PR.

@kennknowles kennknowles deleted the ParDo-Supplier branch November 10, 2016 03:10
damccorm added a commit that referenced this pull request Dec 9, 2022
…rs (#23160)

* Updated java_tests workflow (#172)

Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>

* Added master changes in java_tests to avoid merge conflicts

* Set runDataflow input to non-required

Co-authored-by: Danny McCormick <dannymccormick@google.com>

* Switching trigger to pull_request (#263)

* Switching trigger to pull_request

* Removing ref from checkout

Co-authored-by: Elias Segundo Antonio <eliassegundo.segundo@gmail.com>
Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>
Co-authored-by: Danny McCormick <dannymccormick@google.com>
Co-authored-by: elink22 <103056145+elink22@users.noreply.github.com>
lostluck pushed a commit to lostluck/beam that referenced this pull request Dec 22, 2022
…rs (apache#23160)

* Updated java_tests workflow (apache#172)

Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>

* Added master changes in java_tests to avoid merge conflicts

* Set runDataflow input to non-required

Co-authored-by: Danny McCormick <dannymccormick@google.com>

* Switching trigger to pull_request (apache#263)

* Switching trigger to pull_request

* Removing ref from checkout

Co-authored-by: Elias Segundo Antonio <eliassegundo.segundo@gmail.com>
Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>
Co-authored-by: Danny McCormick <dannymccormick@google.com>
Co-authored-by: elink22 <103056145+elink22@users.noreply.github.com>
pl04351820 pushed a commit to pl04351820/beam that referenced this pull request Dec 20, 2023
* docs: add documentation for documentsnapshot class

* chore: drop superseded docs fix

See PR apache#278.

Co-authored-by: Tres Seaver <tseaver@palladion.com>
Co-authored-by: Christopher Wilcox <crwilcox@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants