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: get rid off blob's ref_map for non-shared objects #9988

Merged
merged 8 commits into from Jul 28, 2016

Conversation

ifed01
Copy link
Contributor

@ifed01 ifed01 commented Jun 28, 2016

Ref_map is built on the fly for onodes that are not shared. Hence provides saving for both serialized and in-memory onode representation.

Signed-off-by: Igor Fedotov ifedotov@mirantis.com

@@ -196,6 +196,7 @@ struct bluestore_blob_t {
}

vector<bluestore_pextent_t> extents;///< raw data position on device
uint32_t compressed_length_orig = 0;///< original length of compressed blob if any
Copy link
Member

Choose a reason for hiding this comment

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

We could just call this 'length'...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer to highlight that this member is applied to compressed blobs only. And we ensure its' persistence in this case only. 'length' hides such semantic IMHO.

@liewegas
Copy link
Member

This looks good to me! It needs a rebase, though.

@liewegas
Copy link
Member

I'm not wild about having to calculate a ref map before we can figure out which refs we are discarding, but I don't see a better way. Perhaps in the future we can keep the (up to date) ref_map in the Blob so that we only generate it on the first put after the Onode is loaded off disk. (Can do that later though..)

@ifed01 ifed01 force-pushed the wip-bluestore-reduce-blob4 branch from 09c03e4 to 5d9480b Compare July 12, 2016 10:49
@ifed01
Copy link
Contributor Author

ifed01 commented Jul 12, 2016

@liewegas: Fixed and rebased

@markhpc
Copy link
Member

markhpc commented Jul 15, 2016

Hi Igor, does this pass "ceph_test_objectstore --gtest_filter=*/2"?

Igor Fedotov added 7 commits July 18, 2016 19:06
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
… track original length for compressed blobs

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
…ernal ref_map for recerence counting

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
…e_extent_ref_map_t

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
…e_onode_t interface to handle reference counting for both ref_map/no ref_map cases in a single place

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
…e_t::punch_hole and others

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@ifed01 ifed01 force-pushed the wip-bluestore-reduce-blob4 branch from 5d9480b to 53af175 Compare July 18, 2016 16:10
@ifed01
Copy link
Contributor Author

ifed01 commented Jul 19, 2016

@markhpc, just rerun the object store test suite - everything is OK

@markhpc
Copy link
Member

markhpc commented Jul 21, 2016

Adding comment to see if I can get jenkins to rerun

@dmick
Copy link
Member

dmick commented Jul 21, 2016

Retest this please

@markhpc
Copy link
Member

markhpc commented Jul 21, 2016

Hi Igor, it looks like this is failing one of the unit tests:

The following tests FAILED:
124 - unittest_bluestore_types (OTHER_FAULT)

Can you please fix and update?

…nt ref_map locations

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@ifed01
Copy link
Contributor Author

ifed01 commented Jul 25, 2016

@markhpc - fixed

@markhpc
Copy link
Member

markhpc commented Jul 27, 2016

While testing this I hit a segfault during ceph_test_objectstore, but on subsequent runs I'm not able to reproduce. This PR may not be responsible for the segfault, but I'm going to try to see if I can track down what's going on before merging.
9988_segfault.txt

@markhpc
Copy link
Member

markhpc commented Jul 28, 2016

Igor mentioned that he has hit this segfault in the past as well and it seems to be related to running out of memory during the test. Given that it has been observed before this PR and that I have not been able to reproduce the segfault in question, I'm going to merge this as it is passing all tests now.

@markhpc markhpc merged commit f8c0e34 into ceph:master Jul 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants