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

Add a banner to mention the data corruption bug #75

Closed
wants to merge 1 commit into from

Conversation

rasky
Copy link

@rasky rasky commented Nov 20, 2019

Until #22 is fixed, this library is using zstd in a way that can cause data corruption, as confirmed by zstd maintainer himself. I think this is critical enough that should be mentioned at the top of README and the Go community should be alerted until the bug is fixed.

Until DataDog#22 is fixed, this library is using zstd in a way that can cause data corruption, as confirmed by zstd maintainer himself. I think this is critical enough that should be mentioned at the top of README and the Go community should be alerted until the bug is fixed
@arl
Copy link

arl commented Nov 29, 2019

IMHO this should be addressed.

We were using this library in production and had to switch to another one because of that.

We only noticed something was off because the sizes of the compressed files were, in some cases, up to 4x bigger than normal. Fortunately we didn't have data corruption.

The README indicates it is stable

@Viq111
Copy link
Collaborator

Viq111 commented Dec 3, 2019

Hi @rasky & @arl,

Sorry I was out last week so getting to review this now.
Indeed, this is a pretty annoying bug that I have been trying to get a consistent reproducible test case for.
As mentioned in #22 (comment), switching to using the ZSTD_compressStream zstd interface will likely fix the issue though I would really like to be able to consistently reproduce first to make 100% we fix both cases (#22 is easy to test #39 is harder)

@rasky Would you have a set of settings where you are able to reproduce the corruption bug consistently ?

Also if anyone is interested in contributing, I only have my free time to contribute to that repository so it's a bit harder for me to undertake big API changes

Sorry again for the wasted time this has caused to anyone
(Going to close that PR in favor of #77 but feel free to continue discussing here)

@Viq111 Viq111 closed this Dec 3, 2019
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