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

[RFC] rgw_file: handle unaligned and unordered write gracefully #15623

Closed
wants to merge 1 commit into from

Conversation

guihecheng
Copy link

Instead of directly return EIO for unaligned and unordered writes,
we could handle them under the neccessary mount -o sync option.

  • for unaligned writes(e.g. dd bs=2000), treat offsets conditionally
  • for unordered writes(large file, buffered IO), pin non-continuous extents
    first, and merge them after extents that fills the gap arrives.

This may serve as a temporary solution until we have more flexible write semantics

Signed-off-by: Gui Hecheng guihecheng@cmss.chinamobile.com

Instead of directly return EIO for unaligned and unordered writes,
we could handle them under the neccessary mount -o sync option.
- for unaligned writes(e.g. dd bs=2000), treat offsets conditionally
- for unordered writes(large file, buffered IO), pin non-continuous extents
first, and merge them after extents that fills the gap arrives.

Signed-off-by: Gui Hecheng <guihecheng@cmss.chinamobile.com>
@xiexingguo xiexingguo added the rgw label Jun 12, 2017
@mattbenjamin mattbenjamin self-assigned this Jun 13, 2017
@mattbenjamin mattbenjamin self-requested a review June 13, 2017 14:12
@guihecheng
Copy link
Author

@mattbenjamin any comments on this one? this help us in tests using vdbench to prevent EIOs.

@HIK-GYK
Copy link

HIK-GYK commented Sep 14, 2017

@guihecheng Hi, my tests showed the code you add in rgw_file.cc is the cause of the error :
////////////////////////////////////////////////////////////////////////////////////////
/* we should keep data ordered,
* do put data until we merged pinned extents */
if (!pinned_extent.empty())
return 0;
//////////////////////////////////////////////////////////////////////////////////////////

thinking about the write scene:
1 off = 0 size = 256K
2 off = 256K size = 256K
3 off = 1024K size = 256K
4 off = 1280K size = 256K
5 off = 512K size = 256K
6 off = 768K size = 256K
...

when the 3rd and 4th write come, we put it in pinned_extent, and when the 5th write come, we can't find 768K in pinned_extent. Then, the code above will judge the pinned_extent.empty(), because of 3rd and 4th write still exist in pinned_extent, 5th write can't be executed. When the 6th write come, data.claim() will add the 6th write. However, data.claim() is difined as:

/////////////////////////////////////////////////////////////////////////////////////////
void buffer::list::claim(list& bl, unsigned int flags)
{
clear();
claim_append(bl,flags);
}
/////////////////////////////////////////////////////////////////////////////////////////

data.claim() will clear the buffer, so the 5th write is cleared before put in the osd pool. That's
what the problem happened.

@guihecheng
Copy link
Author

@HIK-GYK oh, thanks for your test and explanation, and I've found that there is a commit from other one to be merged #17809.
That detects similar problems and use a similar approach to address it, maybe it helps ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants