Skip to content

Default write empty chunks false #951

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

Conversation

joshmoore
Copy link
Member

Re-opening #853 from another branch to see if that fixes the github action jobs.

@jakirkham
Copy link
Member

jakirkham commented Jan 26, 2022

Also trying a no change PR ( #952 ) to see if this is a CI issue outside of these changes

@joshmoore
Copy link
Member Author

Fewer are hanging but not nothing. 😵‍💫

@jakirkham
Copy link
Member

It would be good to get a clearer idea on why that is happening. Did we manage to get a log that we could look at? Waited for a while to see if GH Actions would include output, but nothing showed up.

@jni
Copy link
Contributor

jni commented Jan 27, 2022

@joshmoore @jakirkham this is very reminiscent of my own misadventures with test_sync, deadlocks, and CI timeouts. @joshmoore want to try your own advice?

#725 (comment)

Usually slow tests are printed at the end, but if they aren't completing we don't get the list! Can you add pytest -sv to the workflow or add pypi.org/project/pytest-timeout so we can get an idea on what's gone sideways?

This commit adds the timeout:

4e7733c (#725)

@joshmoore
Copy link
Member Author

Heh. I think I thought that that was now done everywhere. Pushing a commit with lots of timeouts here now.

@joshmoore joshmoore force-pushed the default-write-empty-chunks-false branch from b30abbb to 8b12549 Compare January 27, 2022 06:37
@jakirkham
Copy link
Member

The thing is this hang showed up very recently. It makes me wonder if there is something we changed or a dependency update in the environment that changed, which is causing issues.

For example fasteners was updated recently. To save writing this again, please read this comment ( #952 (comment) ).

@joshmoore
Copy link
Member Author

Interesting that not even timeout seems to be working here.

joshmoore added a commit to joshmoore/zarr-python that referenced this pull request Jan 27, 2022
@joshmoore joshmoore force-pushed the default-write-empty-chunks-false branch from 666b500 to 080a854 Compare January 27, 2022 07:35
@joshmoore
Copy link
Member Author

0.16.3 seemes safe, @jakirkham. Trying 0.17.1 here now.

@joshmoore
Copy link
Member Author

0.17.1 seems to be hanging on 3.9 != 1.21.0 as well.

@joshmoore
Copy link
Member Author

Pushed the commits up to the 0.16.3 pin to #853.

@jakirkham
Copy link
Member

The fact that timeout doesn't work makes me wonder if the hang is in C. Is it possible this is NumPy related? Is NumPy 1.22 hanging somewhere? What if we pinned to NumPy 1.20 in the jobs currently skipping 1.21?

@joshmoore joshmoore force-pushed the default-write-empty-chunks-false branch from a9fd445 to f222cbe Compare January 27, 2022 13:10
@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #951 (a9fd445) into master (e15001b) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head a9fd445 differs from pull request most recent head 285998e. Consider uploading reports for the commit 285998e to get more accurate results

@@            Coverage Diff             @@
##           master     #951      +/-   ##
==========================================
- Coverage   99.94%   99.94%   -0.01%     
==========================================
  Files          32       32              
  Lines       11225    11134      -91     
==========================================
- Hits        11219    11128      -91     
  Misses          6        6              
Impacted Files Coverage Δ
setup.py 0.00% <ø> (ø)
zarr/_storage/absstore.py 100.00% <ø> (ø)
zarr/core.py 100.00% <ø> (ø)
zarr/creation.py 100.00% <ø> (ø)
zarr/util.py 100.00% <ø> (ø)
zarr/n5.py 100.00% <0.00%> (ø)
zarr/storage.py 100.00% <0.00%> (ø)
zarr/indexing.py 100.00% <0.00%> (ø)
zarr/hierarchy.py 100.00% <0.00%> (ø)
... and 11 more

@joshmoore joshmoore force-pushed the default-write-empty-chunks-false branch from f222cbe to a062a85 Compare January 27, 2022 13:17
@jakirkham
Copy link
Member

Yeah sorry I think you may be right. Going with 0.16 seems like the right idea. There were deadlock issues in previous 0.17 releases it seems

@joshmoore
Copy link
Member Author

Going with 0.16 seems like the right idea.

Yeah, I was feeling pretty good about it until 080a854 timed out again. But perhaps that PR is somehow "tainted".

@jakirkham
Copy link
Member

Maybe we should run tests with more verbosity? Perhaps that points out which test hangs so we can investigate further

@jakirkham
Copy link
Member

Power cycling for CI

@jakirkham jakirkham closed this Feb 4, 2022
@jakirkham jakirkham reopened this Feb 4, 2022
@jakirkham
Copy link
Member

jakirkham commented Feb 4, 2022

Looks like this is going to fail without the NumPy upgrade ( #955 ). Or perhaps the new NumPy conflicts with pinnings we tried here?

@joshmoore
Copy link
Member Author

Closing along with #952

Looks like the scare is over.

@joshmoore joshmoore closed this Feb 4, 2022
@joshmoore joshmoore deleted the default-write-empty-chunks-false branch February 4, 2022 10:03
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.

4 participants