Enable zstd in tests #8288
Closed
Enable zstd in tests #8288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
30 tasks
No tests were run or test report is not availableTest coverage report is not availableThe comment gets automatically updated with the latest test results
4d8c7a3 at 2024-07-12T02:23:25.217Z :recycle: |
arpad-m
force-pushed
the
arpad/compression_6
branch
from
86b673e to
9067bfe
Compare
|
I'm really curious about why this all passes without #8302 -- maybe we have a configuration issue where tests are running without vectored reads. Let's figure this out before proceeding. |
arpad-m
force-pushed
the
arpad/compression_6
branch
from
3daf23c to
4739eba
Compare
Yeah it's really strange. For shining more light on the issue, I've
|
CI tests and benchmarks use vectored read by default. We even print out the read path config at startup (example from a random recent ci run): |
|
So after artificially introducing a corruption in this PR during reading of compressed fullpage images (just setting all returned data to the same byte), I don't see any pytest failures. On the other hand, when making vectored return 0'd out data #8324, there is a lot of failures. As a conclusion, this means that we didn't notice the missing vectored read implementation not because there is no vectored read testing, but because there isn't any tests that cause us to compress data. This might be due to a gap in test coverage, where there is no well compressible full page images in image layers, or maybe also due to the method I chose in this PR to enable zstd: maybe it simply doesn't work? |
arpad-m
added a commit
that referenced
this pull request
Jul 11, 2024
|
Found the culprit: it was |
|
This is the newest CI run. Sadly no allure report available because of timeouts. |
|
Manually extracted list, 76 failures: |
|
extracted error for |
|
closing this, will open a new branch for further experiments. |
arpad-m
added a commit
that referenced
this pull request
Jul 18, 2024
Successor of #8288 , just enable zstd in tests. Also adds a test that creates easily compressable data. Part of #5431 --------- Co-authored-by: John Spray <[email protected]> Co-authored-by: Joonas Koivunen <[email protected]>
problame
pushed a commit
that referenced
this pull request
Jul 22, 2024
Successor of #8288 , just enable zstd in tests. Also adds a test that creates easily compressable data. Part of #5431 --------- Co-authored-by: John Spray <[email protected]> Co-authored-by: Joonas Koivunen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enables zstd compression in the testsuite. Builds on #8281.
Part of #5431