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

os/bluestore: attach csum for compressed blobs #37168

Merged
merged 1 commit into from Oct 14, 2020

Conversation

ifed01
Copy link
Contributor

@ifed01 ifed01 commented Sep 15, 2020

Fixes: https://tracker.ceph.com/issues/47475
Signed-off-by: Igor Fedotov ifedotov@suse.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

Fixes: https://tracker.ceph.com/issues/47475
Signed-off-by: Igor Fedotov <ifedotov@suse.com>
@aclamk
Copy link
Contributor

aclamk commented Sep 21, 2020

@ifed01 But hasn't it been an assumption that compressors will sanitize its content?
You think its ability for error detection might not be sufficient?

@ifed01
Copy link
Contributor Author

ifed01 commented Sep 21, 2020

@ifed01 But hasn't it been an assumption that compressors will sanitize its content?
You think its ability for error detection might not be sufficient?

The following ticket has inspired me for this fix: https://tracker.ceph.com/issues/46490
While the root cause for disk read failures is still unclear there one of their consequence is an assertion during decoding of improperly read (and hence invalid) compression header.
Generally I think that's a good idea to have csum for this sort of data, I presume the overhead is neglectable.

Copy link
Contributor

@aclamk aclamk left a comment

Choose a reason for hiding this comment

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

Tested, works.
This is good improvement.

@tchaikov tchaikov merged commit b43f26f into ceph:master Oct 14, 2020
@ifed01 ifed01 deleted the wip-ifed-fix-compress-csum branch October 14, 2020 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants