Skip to content

os/bluestore: fix do_write_v2 extentmap compress issue#61762

Merged
liu-chunmei merged 1 commit intoceph:mainfrom
liu-chunmei:compress_extentmap
Feb 18, 2025
Merged

os/bluestore: fix do_write_v2 extentmap compress issue#61762
liu-chunmei merged 1 commit intoceph:mainfrom
liu-chunmei:compress_extentmap

Conversation

@liu-chunmei
Copy link
Contributor

@liu-chunmei liu-chunmei commented Feb 11, 2025

do_write_v2 insert extentmap by aligned offset which is adjusted in writer.do_write, if use original offset will cause extentmap can't do compress.

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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 dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

do_write_v2 insert extentmap by aligned offset which is adjusted in
writer.do_write, if use original offset will cause extentmap can't do
compress.

Signed-off-by: chunmei liu <chunmei.liu@ibm.com>
@liu-chunmei
Copy link
Contributor Author

jenkins test make check

1 similar comment
@aclamk
Copy link
Contributor

aclamk commented Feb 18, 2025

jenkins test make check

@@ -17521,9 +17521,6 @@ int BlueStore::_do_write_v2(
o->extent_map.fault_range(db, offset, length);
Copy link
Contributor

@aclamk aclamk Feb 18, 2025

Choose a reason for hiding this comment

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

I think the PR fix is ok, but not what troubles me is that we do some "around-searching" for blobs and extents nearby in _maybe_reuse_blob etc.
But we do fault_range on range offset-length.
I guess we need to make sure that one of conditions hold:
a) we never produce mapping that goes outside the range offset-length
or
b) we expand fault_range so much that we will not look outside
I think b) is better as there was hard limit of lookaround = min_alloc_size.

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.

Approved.
Comments are likely a material for another PR.

@liu-chunmei liu-chunmei merged commit e52e550 into ceph:main Feb 18, 2025
13 of 16 checks passed
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.

2 participants