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

[air] Randomize block order by default to avoid hotspots #25870

Merged
merged 9 commits into from
Jun 30, 2022

Conversation

ericl
Copy link
Contributor

@ericl ericl commented Jun 16, 2022

Why are these changes needed?

Enable block order randomization by default to avoid ingest hotspots when running concurrent trials.

if config.use_stream_api:
dataset = dataset.random_shuffle_each_window()
else:
dataset = dataset.random_shuffle()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bug fix!

Comment on lines 140 to 141
if conf.randomize_block_order:
dataset = dataset.randomize_block_order()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add randomize_block_order_each_window for the pipelined case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was debating that. I guess we may as well in case it helps with randomization, even though it doesn't help with hotspots.

@ericl ericl added the do-not-merge Do not merge this PR! label Jun 16, 2022
@ericl
Copy link
Contributor Author

ericl commented Jun 16, 2022

Putting a hold on this, since we identified that this is breaking stage fusion from read->map.

@ericl
Copy link
Contributor Author

ericl commented Jun 24, 2022

Closing this for now.

@ericl ericl closed this Jun 24, 2022
@ericl ericl reopened this Jun 24, 2022
@ericl ericl removed the do-not-merge Do not merge this PR! label Jun 30, 2022
@ericl
Copy link
Contributor Author

ericl commented Jun 30, 2022

This should be good to go now that fusion is fixed.

@ericl ericl merged commit 3b1948e into ray-project:master Jun 30, 2022
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.

3 participants