Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Data] Skip execution for LogicalPlans with only InputData op #41597

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

scottjlee
Copy link
Contributor

@scottjlee scottjlee commented Dec 4, 2023

Why are these changes needed?

  • After [Data] Enable read-only Datasets to be executed on new execution backend #41466, all Datasets are executed using the new streaming executor backend. There is an edge case that was not caught related to MaterializedDatasets, where executing the already materialized dataset caused extraneous (empty) metrics to be registered from these newly created datasets. This PR covers this edge case by skipping execution in the case where the Dataset's logical plan consists of only an InputData operator.
  • Also enables Data dashboard tests to run for any data-related change, not just for dashboard related changes.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: Scott Lee <sjl@anyscale.com>
@aslonnie
Copy link
Collaborator

aslonnie commented Dec 5, 2023

I will force merge this for you.

@aslonnie aslonnie merged commit 237505b into ray-project:master Dec 5, 2023
12 of 16 checks passed
Copy link
Contributor

@c21 c21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late LGTM.

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.

None yet

5 participants