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: pass strict flag to bluestore_blob_use_tracker_t::equal() #15705

Merged
merged 1 commit into from Sep 4, 2017

Conversation

xiexingguo
Copy link
Member

@xiexingguo xiexingguo commented Jun 15, 2017

And if that flag is true, we'll do a strict equivalence comparison instead.

Signed-off-by: xie xingguo xie.xingguo@zte.com.cn

@xiexingguo xiexingguo force-pushed the wip-fix-blob-tracker-check branch 2 times, most recently from fb2f7ea to c25cd8a Compare June 15, 2017 08:29
@ifed01
Copy link
Contributor

ifed01 commented Jun 15, 2017

I'm disagree with the initial assumption that <1> isn't equal to <2>. In fact <2> doesn't have that actual range information [1..1001] - it contains total referenced bytes counter only. And that amount of referenced bytes equivalent (weakly?) to the one at <1> hence returning true. I.e. equal method returns true if there is a chance that two trackers describe the same used_range rather than strict comparison. Probably it makes sense to change the function name though..

@xiexingguo
Copy link
Member Author

xiexingguo commented Jun 15, 2017

I.e. equal method returns true if there is a chance that two trackers describe the same used_range rather than strict comparison.

I see the only caller is fsck(), which means we use this method to measure the same blob, so I guess we'd better narrow the constraints down.

@ifed01
Copy link
Contributor

ifed01 commented Jun 19, 2017

I'd prefer to leave current comparison mode as well. May be we can have a boolean flag for equal() call to request strict/weak equivalence comparison.

@xiexingguo xiexingguo changed the title os/bluestore: fix bluestore_blob_use_tracker_t::equal() os/bluestore: pass strict flag to bluestore_blob_use_tracker_t::equal() Jun 22, 2017
@xiexingguo
Copy link
Member Author

I'd prefer to leave current comparison mode as well. May be we can have a boolean flag for equal() call to request strict/weak equivalence comparison.

Updated!

@@ -488,7 +488,8 @@ void bluestore_blob_use_tracker_t::split(
}

bool bluestore_blob_use_tracker_t::equal(
const bluestore_blob_use_tracker_t& other) const
const bluestore_blob_use_tracker_t& other,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add some UT for that

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM.

@xiexingguo
Copy link
Member Author

xiexingguo commented Jun 29, 2017

Could you please add some UT for that

Sure, added.

And if that flag is true, we'll do a strict equivalence comparison instead.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
@xiexingguo
Copy link
Member Author

retest this please

1 similar comment
@xiexingguo
Copy link
Member Author

retest this please

@xiexingguo xiexingguo merged commit aa27ac7 into ceph:master Sep 4, 2017
@xiexingguo xiexingguo deleted the wip-fix-blob-tracker-check branch September 4, 2017 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants