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

fix: Do not close connection twice in DigestWithOptions #21860

Merged
merged 2 commits into from
Jul 16, 2021

Conversation

danxmoran
Copy link
Contributor

Backports #21662

davidby-influx and others added 2 commits July 16, 2021 10:28
tsm1.DigestWithOptions closes its network connection
twice. This may cause broken pipe errors on concurrent
invocations of the same procedure, by closing a reused
i/o descriptor. This fix also captures errors from TSM
file closures, which were previously ignored.
Copy link
Contributor

@williamhbaker williamhbaker left a comment

Choose a reason for hiding this comment

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

👍

@@ -22,7 +22,7 @@ type DigestOptions struct {

// DigestWithOptions writes a digest of dir to w using options to filter by
// time and key range.
func DigestWithOptions(dir string, files []string, opts DigestOptions, w io.WriteCloser) error {
func DigestWithOptions(dir string, files []string, opts DigestOptions, w io.WriteCloser) (err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we use digesting for something in 2.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm it looks like no (the call chain traces to ShardDigest, which isn't called anywhere). Think this should be closed? Or merged to keep things as in-sync as possible while it's easy?

@danxmoran danxmoran merged commit fc23396 into 2.0 Jul 16, 2021
@danxmoran danxmoran deleted the dm-backport-no-double-close branch July 16, 2021 17:06
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