Skip to content

WIP: osd, librados: chunk scrub for the dedup tier#24230

Closed
myoungwon wants to merge 3 commits intoceph:masterfrom
myoungwon:wip-chunk-scrub
Closed

WIP: osd, librados: chunk scrub for the dedup tier#24230
myoungwon wants to merge 3 commits intoceph:masterfrom
myoungwon:wip-chunk-scrub

Conversation

@myoungwon
Copy link
Member

@myoungwon myoungwon commented Sep 24, 2018

please refer to #22987

  • Implement chunk-scrub for a single object
  • Unit test cases, stress test cases
  • Implement background chunk-scrub
  • Stress test cases for background chunk-scrub

Signed-off-by: Myoungwon Oh omwmw@sk.com

@myoungwon myoungwon force-pushed the wip-chunk-scrub branch 3 times, most recently from 10d97e4 to 2b6d242 Compare September 30, 2018 15:19
@myoungwon myoungwon force-pushed the wip-chunk-scrub branch 4 times, most recently from 4bdc4e8 to 16193d1 Compare October 7, 2018 08:21
Signed-off-by: Myoungwon Oh <omwmw@sk.com>
Signed-off-by: Myoungwon Oh <omwmw@sk.com>
Signed-off-by: Myoungwon Oh <omwmw@sk.com>
@myoungwon
Copy link
Member Author

@liewegas Before starting the next work, I want to get a feedback for changes I made.

As you know, the key idea behind of reference counting is false-positive, which means (manifest object (no ref), chunk object(has ref)) can be possible instead of (manifest object (has ref), chunk 1(no ref)). So, if we want to add chunk-scrub, it would be enough to add scrub operation in the chunk pool.

I have implemented a chunk-scrub op and functions in the osd for a single chunk object.
Can you review this? Also, I have a plan to implement a background chunk-scrub (iterating all of the chunk object and do chunk-scrub). what do you think?

@liewegas
Copy link
Member

liewegas commented Oct 8, 2018

My first instinct is to implement the scrubbing as a tool that lives outside of the OSD, actually--it avoids the maintenance burden of all of in-flight scrub ops and concerns around large objects counts. What if instead we just provide the rados ops to check if a chunk is referenced (like you have) and another one that returns the whole list of backpointers?

Separately, I had a good discussion with some of the dedup folks here at Red Hat; sending an email to the list about that.

myoungwon added a commit to myoungwon/ceph that referenced this pull request Oct 21, 2018
- manifest unset op to foo object
 - remove manifest flag
 - commit
 - send an ack to a client
 - send decrement mesages ("chunk_put") to old chunks

Current unit test(ManifestUnset) send "chunk_read" command (to foo-chunk)
in order to see whether chunk's reference count is decreased.
But, as described above, "chunk_read" event can be triggered after a client
(test application) receives an ack. Therefore, there is a corner case
such as an OSD (in chunk pool) receives "chunk_read" first instead of "chunk_put"

Reference count model of dedup/tiering is based on false-positive (ceph#24230).
So decreasing reference count is not guaranteed. If reference mismatch occur,
chunk-scrub(this is WIP) will fix it.
One guaranteed thing is that existing manifest flag is removed.

So, the solution of this commit is just re-send unset op, and then
chenk that return value is -EOPNOTSUPP (this means manifest flags is removed).

Signed-off-by: Myoungwon Oh <omwmw@sk.com>
myoungwon added a commit to myoungwon/ceph that referenced this pull request Oct 21, 2018
- manifest unset op to foo object
 - remove manifest flag
 - commit
 - send an ack to a client
 - send decrement mesages ("chunk_put") to old chunks

Current unit test(ManifestUnset) send "chunk_read" command (to foo-chunk)
in order to see whether chunk's reference count is decreased.
But, as described above, "chunk_read" event can be triggered after a client
(test application) receives an ack. Therefore, there is a corner case
such as an OSD (in chunk pool) receives "chunk_read" first instead of "chunk_put"

Reference count model of dedup/tiering is based on false-positive (ceph#24230).
So decreasing reference count is not guaranteed. If reference mismatch occur,
chunk-scrub(this is WIP) will fix it.
One guaranteed thing is that existing manifest flag is removed.

So, the solution of this commit is just re-send unset op, and then
chenk that return value is -EOPNOTSUPP (this means manifest flags is removed).

Fixes: http://tracker.ceph.com/issues/24485
Signed-off-by: Myoungwon Oh <omwmw@sk.com>
myoungwon added a commit to myoungwon/ceph that referenced this pull request Oct 21, 2018
- manifest unset op to foo-chunk object
 - remove manifest flag
 - commit
 - send an ack to a client
 - send decrement mesages ("chunk_put") to old chunks (bar-chunk)

Current unit test(ManifestUnset) send "chunk_read" command (to bar-chunk)
in order to see whether chunk's reference count is decreased.
But, as described above, "chunk_read" event can be triggered after a client
(test application) receives an ack. Therefore, there is a corner case
such as bar-chunk (in chunk pool) receives "chunk_read" first instead of "chunk_put"

Reference count model of dedup/tiering is based on false-positive (ceph#24230).
So decreasing reference count is not guaranteed. If reference mismatch occur,
chunk-scrub(this is WIP) will fix it.
One guaranteed thing is that existing manifest flag is removed.

So, the solution of this commit is just re-send unset op, and then
chenk that return value is -EOPNOTSUPP (this means manifest flags is removed).

Fixes: http://tracker.ceph.com/issues/24485
Signed-off-by: Myoungwon Oh <omwmw@sk.com>
myoungwon added a commit to myoungwon/ceph that referenced this pull request Oct 21, 2018
- manifest unset op to foo-chunk object
 - remove manifest flag
 - commit
 - send an ack to a client
 - send decrement mesages ("chunk_put") to old chunks (bar-chunk)

Current unit test(ManifestUnset) send "chunk_read" command (to bar-chunk)
in order to see whether chunk's reference count is decreased.
But, as described above, "chunk_read" event can be triggered after a client
(test application) receives an ack. Therefore, there is a corner case
such as bar-chunk (in chunk pool) receives "chunk_read" first instead of "chunk_put"

Reference count model of dedup/tiering is based on false-positive (ceph#24230).
So decreasing reference count is not guaranteed. If reference mismatch occur,
chunk-scrub (this is WIP) will fix it.
One guaranteed thing is that existing manifest flag is removed.

So, the solution of this commit is just re-send unset op, and then
chenk that return value is -EOPNOTSUPP (this means manifest flags is removed).

Fixes: http://tracker.ceph.com/issues/24485
Signed-off-by: Myoungwon Oh <omwmw@sk.com>
@myoungwon myoungwon mentioned this pull request Oct 29, 2018
8 tasks
@stale
Copy link

stale bot commented Dec 7, 2018

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Dec 7, 2018
@stale
Copy link

stale bot commented Apr 22, 2019

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

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