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

Fix channel buffer size in namespace datastore to use KeysOnlyBufSize #52

Merged
merged 2 commits into from
Nov 14, 2016

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Nov 14, 2016

No description provided.

@kevina kevina added the status/in-progress In progress label Nov 14, 2016
@kevina
Copy link
Contributor Author

kevina commented Nov 14, 2016

See ipfs/kubo#3376.

I better way would be to use something like make(chan dsq.Result, cap(qr.Next())) but for some reason that cap(qr.Next()) is returning 0 when it should be returning 128 (i.e. dsq.KeysOnlyBufSize) so I manually set the buffer size based on it it was a keys-only query.

@kevina
Copy link
Contributor Author

kevina commented Nov 14, 2016

Note the test failure is due to the use of go 1.6.3 that no longer appears to be supported with the latest version of go-datastore.

@Kubuxu
Copy link
Member

Kubuxu commented Nov 14, 2016

Can you update the travis file to use 1.7.3 and tip.

@kevina
Copy link
Contributor Author

kevina commented Nov 14, 2016

@Kubuxu

Can you update the travis file to use 1.7.3 and tip.

Done.

@whyrusleeping
Copy link
Member

Sweet, LGTM. I think the biggest problem here is that we're trying to use a channel as an iterator and thats just inherently slow. We can look at switching up interfaces later, but the changes being made so far look very promising

@whyrusleeping whyrusleeping merged commit 76c8200 into master Nov 14, 2016
@whyrusleeping whyrusleeping deleted the kevina/namespace-bufsize branch November 14, 2016 21:36
@whyrusleeping whyrusleeping removed the status/in-progress In progress label Nov 14, 2016
@kevina kevina self-assigned this Nov 16, 2016
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.

3 participants