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 #21659

Merged
merged 2 commits into from
Jun 10, 2021

Conversation

davidby-influx
Copy link
Contributor

@davidby-influx davidby-influx commented Jun 10, 2021

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.

Closes #21656

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.

Closes #21656
@gwossum
Copy link
Member

gwossum commented Jun 10, 2021

Previously any errors from closing tsmFiles were not returned to the caller. Will returning the error now cause issues (assuming the tsmFile.Close() can fail)?

@davidby-influx
Copy link
Contributor Author

My belief is that if TSMFile.Close() causes errors, those are errors we want to know about, because they are failures of things like unix.Munmap() and indicate serious problems in our OS or filesystem. I believe the code was written to ignore them because the chance of failure was very low, not because it was high.

@gwossum
Copy link
Member

gwossum commented Jun 10, 2021

After conferring with David Norton, we should prefer to return errors from defer.

Copy link
Member

@gwossum gwossum left a comment

Choose a reason for hiding this comment

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

Looks good to me

@davidby-influx davidby-influx merged commit bce6553 into master-1.x Jun 10, 2021
@davidby-influx davidby-influx deleted the DSB_digest_close branch June 10, 2021 19:44
davidby-influx added a commit that referenced this pull request Jun 10, 2021
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.

Closes #21656

(cherry picked from commit bce6553)
davidby-influx added a commit that referenced this pull request Jun 10, 2021
)

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.

Closes #21656

(cherry picked from commit bce6553)

Closes #21660
davidby-influx added a commit that referenced this pull request Jun 15, 2021
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.

Closes #21656

(cherry picked from commit bce6553)
davidby-influx added a commit that referenced this pull request Jun 15, 2021
)

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.

Closes #21656

(cherry picked from commit bce6553)

Closes #21657
chengshiwen pushed a commit to chengshiwen/influxdb that referenced this pull request Aug 11, 2024
…1659)

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.

Closes influxdata#21656
chengshiwen pushed a commit to chengshiwen/influxdb that referenced this pull request Aug 27, 2024
…1659)

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.

Closes influxdata#21656
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tsm1.DigestWithOptions() calls Close() twice on the network connection
2 participants