Closed jrbourbeau closed 1 year ago
We'd like to include https://github.com/dask/distributed/pull/6996 as part of the release. It's limited to changes within UCX usage in Dask
Graham also fixed the CI issue we saw with numba serialization: https://github.com/dask/distributed/pull/7089
I'd love to see https://github.com/dask/distributed/pull/7075 in that release.
We'd like to include https://github.com/dask/distributed/pull/6996 as part of the release. It's limited to changes within UCX usage in Dask
Sounds good. @quasiben could I ask you to shepherd those changes through?
I'd love to see https://github.com/dask/distributed/pull/7075 in that release.
I just pinged Gabe and Guido to see if they have bandwidth to review
@hendrikmakait can you remind me, have we run benchmarking on https://github.com/dask/distributed/pull/7075 and/or are we confident there are no behavioral changes, just a different way of deriving an identical occupancy number? I've started reviewing and though everything looks fine, it's just a substantial change to a delicate part of scheduling (occupancy). I'd be slightly wary of releasing it without some integration testing first.
Sounds good. @quasiben could I ask you to shepherd those changes through?
+1
@hendrikmakait can you remind me, have we run benchmarking on dask/distributed#7075 and/or are we confident there are no behavioral changes, just a different way of deriving an identical occupancy number? I've started reviewing and though everything looks fine, it's just a substantial change to a delicate part of scheduling (occupancy). I'd be slightly wary of releasing it without some integration testing first.
Agreed, some A/B testing would make me feel a lot more comfortable.
@hendrikmakait can you remind me, have we run benchmarking on dask/distributed#7075 and/or are we confident there are no behavioral changes, just a different way of deriving an identical occupancy number? I've started reviewing and though everything looks fine, it's just a substantial change to a delicate part of scheduling (occupancy). I'd be slightly wary of releasing it without some integration testing first.
IIRC, @fjetter did some preliminary performance evaluation on #7030, but I am not sure how much has changed since. I agree, running benchmarks after sorting out code review issues should happen before merging.
With respect to behavioral changes, I think we are pretty much guaranteed to have some (e.g., we do not have the non-deterministic recalculation of occupancy anymore). The question should rather be whether these impact performance.
Update: We are currently running an A/B test to assess dask/distributed#7075, but we should not hold up the release waiting for it.
Okay, I'm going to start pushing out the release. The two PRs @quasiben mentioned as in -- let's include https://github.com/dask/distributed/pull/7075 in the next release (apologies @hendrikmakait)
+1 Thanks @jrbourbeau !
2022.9.2 is out on PyPI
conda-forge package and docker images are published. Thanks all!
Best effort
Try to close before the release but will not block the release
Blocker
Issues that would cause us to block and postpone the release if not fixed
Comments
cc @ian-r-rose @gjoseph92 @crusaderky @quasiben @jakirkham