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

osd: fix unordered read bug (for chunked object) #19464

Merged
merged 3 commits into from Dec 16, 2017

Conversation

myoungwon
Copy link
Member

@myoungwon myoungwon commented Dec 13, 2017

The current implementation for chunked object only supports
proxy_read(if offset is within range) and write(local write)
In this case, a read request can be handled before a write request.
This commit prevents unordered read processing because
proxy_read() will be executed if the chunk is missing state.
If chunked object has been overwritten, its state will not be missing.

Fixes: http://tracker.ceph.com/issues/22369
Signed-off-by: Myoungwon Oh omwmw@sk.com

The current implementation for chunked object only supports
proxy_read(if offset is within range) and write(local write)
In this case, a read request can be handled before a write request.
This commit prevents unordered read processing because
proxy_read() will be executed if the chunk is missing state.
If chunked object has been overwritten, its state will not be missing.

Fixes: http://tracker.ceph.com/issues/22369
Signed-off-by: Myoungwon Oh <omwmw@sk.com>
@myoungwon
Copy link
Member Author

retest this please

current chunked object and ChunkReadOp are
only for the read case.
write op and promote_object() still be tested without ChunkReadOp
by another ceph_test_rados in the same test suite (with --set_chunk)

Signed-off-by: Myoungwon Oh <omwmw@sk.com>
This commit prevents unnecessary stat that
invokes promote_object()
promote_object() makes object clean state

Signed-off-by: Myoungwon Oh <omwmw@sk.com>
@tchaikov
Copy link
Contributor

@myoungwon i'd suggest contributor comment on the PR to explain what change he/she made if the PR is updated after it's reviewed/approved. otherwise we can hardly tell if the test is valid or not.

@myoungwon
Copy link
Member Author

@tchaikov Sorry, The main purpose of updated commit is to make a chunk MISSING state when ChunkReadOp test is executed. Since stat and write op make a chunk CLEAN state (through promote_object), this causes local read (not chunk_proxy_read).

If flush() is implemented (#19294), combination test (promote , flush, write and read ...) will be added.
Current implementation mainly implements chunk_proxy_read() and promote_object() for chunks.

Do you need more explanation?

@tchaikov
Copy link
Contributor

tchaikov commented Dec 15, 2017

@myoungwon i have not reviewed this change, so it's up to the reviewer. i just wanted to understand if it's safe to merge. because @liewegas has approved your PR. but it's risky to merge as it is without confirming with you.

@myoungwon myoungwon merged commit 9d2456f into ceph:master Dec 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants