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

Allow ordering by key on mount query #121

Closed
wants to merge 3 commits into from

Conversation

michaelavila
Copy link
Contributor

This work was done to support https://github.com/ipfs/go-ipfs/pull/6068/files#r266291056.

Overview

Mount datastore used to not support key ordering in queries, now it does.

@ghost ghost assigned michaelavila Mar 18, 2019
@ghost ghost added the status/in-progress In progress label Mar 18, 2019
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Looking at this change has made me realize that I made a mistake when I said this was going to be really easy. We can have the following mounts:

  • /foo
  • /foo/bar

In that case, we need to process some of /foo until we hit /foo/bar, then we need to process /foo/bar, then we need to go back to /foo.

There are a bunch of ways to handle this but, IMO, the simplest is to:

  1. Query in parallel.
  2. Put the next item in each query result (along with a pointer to the query result) into a priority queue.
  3. Pull the first item off the priority queue, replacing it with the next item from the query.

This shouldn't be difficult, just slightly more complicated.

@michaelavila
Copy link
Contributor Author

@Stebalien that makes a lot of sense, we hadn't considered such mounts. I'll work on a change. Thanks.

@michaelavila
Copy link
Contributor Author

michaelavila commented Mar 20, 2019

I've added a simplistic and poorly factored solution that addresses the problem, but probably won't be merged as is. A more reasonable solution, I think, is to (1) make the queries all in parallel, and (2) do a k-way merge on the results (which is what I think @Stebalien suggested in IRC).

I'm looking into the better solution now, but I have a proposal on the PR for the original work (https://github.com/ipfs/go-ipfs/pull/6068/files#r267463222) to consider pursuing this as its own task in order to get that other work merged.

@michaelavila
Copy link
Contributor Author

Superseded by #124

@ghost ghost removed the status/in-progress In progress label Mar 22, 2019
@michaelavila michaelavila deleted the features/provide-roots-on-add-and-pin branch March 25, 2019 15:08
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 this pull request may close these issues.

None yet

2 participants