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

[zstd_stream] Add Writer.Flush() #86

Merged
merged 3 commits into from
Aug 17, 2020
Merged

[zstd_stream] Add Writer.Flush() #86

merged 3 commits into from
Aug 17, 2020

Conversation

delthas
Copy link
Contributor

@delthas delthas commented Aug 2, 2020

  • Add a fix for Writer.Close.
  • Add Writer.Flush()

See commits messages for detailed info.

Previously, if there was any remaining data to be written to the
underlying Writer (which was almost always the case, since zstd tries to
buffer blocks and frames as much as possible), and there was an
underlying error when doing a Write on the underlying io.Writer, no
error was returned from Close.

This was an issue because clients would believe the stream was correctly
written in its entirety even though the last bytes were not written.

This fixes the issue by catching these errors, closing the zstd stream
and returning the i/o error.
There are two levels of buffering in the library:
- zstd buffers its data by not closing zstd blocks until a specific call
  to ZSTD_compressStream2 with ZSTD_e_flush or ZSTD_e_end is made
- the Go wrapper buffers the input data to its own buffer

Previously there was no way to flush these two buffers without closing
the stream.

This adds a way to flush these streams in order for any reader that
wants to read was written to the stream until that point, to do so
without waiting for the entire stream to be closed.
Copy link
Collaborator

@Viq111 Viq111 left a comment

Choose a reason for hiding this comment

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

Hello @delthas

Thanks for your contribution!
Sorry for the delay, I was out the last weeks but the code lgtm.

Thanks also for fixing the writer.Close() not returning errors on underlying writer failure

@Viq111 Viq111 merged commit 4b8fdba into DataDog:1.x Aug 17, 2020
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.

2 participants