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

Fetch OMap keys in bulk that one by one #434

Closed
ShyamsundarR opened this issue Jun 14, 2019 · 7 comments · Fixed by #1071
Closed

Fetch OMap keys in bulk that one by one #434

ShyamsundarR opened this issue Jun 14, 2019 · 7 comments · Fixed by #1071
Assignees

Comments

@ShyamsundarR
Copy link
Contributor

In function GetObjectUUIDData we fetch multiple keys from the same OMap, this can be optimized to fetch all keys in one call, and hence save a network roundtrip in fetching these one by one.

This should speed up operations around snapshot management, as the second key is fetched (at present) when snapshots are in play.

The intention is to use list equivalents to fetch all keys, rather dump one key at a time into the output file.

@humblec
Copy link
Collaborator

humblec commented Aug 21, 2019

Moving out of release-1.2.0 as this is yet to be started.

@phlogistonjohn
Copy link
Contributor

@nixpanic @ShyamsundarR I'm interested in taking a stab at this. It looks nicely self contained and a good way to start re-learning ceph-csi again. Thoughts?

@ShyamsundarR
Copy link
Contributor Author

@nixpanic @ShyamsundarR I'm interested in taking a stab at this. It looks nicely self contained and a good way to start re-learning ceph-csi again. Thoughts?

More power to you :) 👍 you would be looking at one call instead of the swarm here I presume, to reduce the call counts. Thanks!

@phlogistonjohn
Copy link
Contributor

I'm happy to look at both. I'll probably start simple with a direct translation of GetOMapValue from cli to go-ceph. Then I'll look into making a "GetMultiOMapValues" kind of thing to see if we can cut down on round-trips.

The existing go-ceph calls, despite a number of issues I already have with them, already support handling multiple omap key/values in one call.

@phlogistonjohn
Copy link
Contributor

Pardon my confusion, but GetObjectUUIDData function appears to be gone now.
Interestingly, the file "./scripts/golangci.yml" appears to be the only thing left referring to it, stating:

  dogsled:
    # voljournal.GetObjectUUIDData currently returns 4 values of which some may
    # not always be useful
    max-blank-identifiers: 3

I have no idea what "dogsled" is but my guess is that this comment is out of date. Shall I remove the comment? I don't have anything to put in its place.

@ShyamsundarR
Copy link
Contributor Author

Pardon my confusion, but GetObjectUUIDData function appears to be gone now.
Interestingly, the file "./scripts/golangci.yml" appears to be the only thing left referring to it, stating:

  dogsled:
    # voljournal.GetObjectUUIDData currently returns 4 values of which some may
    # not always be useful
    max-blank-identifiers: 3

I have no idea what "dogsled" is but my guess is that this comment is out of date. Shall I remove the comment? I don't have anything to put in its place.

I did not know this existed in that file. We can remove it, as I fixed that up to return a structure.

Apparently (googling states) dogsled is: "dogsled: Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f())" (from here), which was true for this function prior to the change to return the struct.

@nixpanic
Copy link
Member

nixpanic commented May 4, 2020

There are a few old commits in my wip/rbd/go-ceph tree. See commits like util: use go-ceph for GetOMapValue(). These need to get adapted to use rbdVol.conn.GetIoctx(poolname) as is done for deleteImage() in PR #862

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

Successfully merging a pull request may close this issue.

4 participants