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

Any use for RangeIterable? #246

Closed
pmezard opened this issue Oct 9, 2015 · 4 comments
Closed

Any use for RangeIterable? #246

pmezard opened this issue Oct 9, 2015 · 4 comments

Comments

@pmezard
Copy link
Contributor

pmezard commented Oct 9, 2015

Hello,

Reading the code I notice the use of RangeIterable in reader.go and other places. No store package with bleve implements this interface. It was apparently used with forestdb backent, which was removed a year ago.

It is useful to preserve the code (assuming there are no tests validating it works)?

@mschoch
Copy link
Contributor

mschoch commented Oct 9, 2015

It was added as a quick test for a performance improvement of ForestDB. The ForestDB KV store is alive and well in the blevex repo.

I'd prefer we not remove it at the moment. I'm in the middle of a complete refactoring/cleanup of the KV store package and API. This is cleaned up as a part of that refactoring, not by removing it, but by giving all KVStores explicit prefix and range iterators (this cleans up the code in a lot of other places as they're not constantly checking the end condition of iteration).

@pmezard
Copy link
Contributor Author

pmezard commented Oct 9, 2015

OK. By curiosity, do you make the refactor publicly or in a branch you intend to merge at some point?

@pmezard pmezard closed this as completed Oct 9, 2015
@mschoch
Copy link
Contributor

mschoch commented Oct 9, 2015

In general if I have long-running branches I try to push them, see the firestorm branch as one example: https://github.com/blevesearch/bleve/tree/firestorm

On that branch we're experimenting with a new indexing scheme, at the moment focused on higher indexing throughput.

In this case, what I thought was going to be simple day-or-two refactor has turned into longer work, and you're right I should push it. My intention is to do all work in the open. I'll try to push it up to github before I stop for the day.

At the moment there are some performance regressions (some expected, and some not). I hope to get these cleaned up soon, and then merge with master. I've been merging in changes from master periodically so I'm not expected any major conflicts at the end.

@mschoch
Copy link
Contributor

mschoch commented Oct 10, 2015

I have pushed the newkvstore branches for bleve and blevex:

https://github.com/blevesearch/bleve/tree/newkvstore
https://github.com/blevesearch/blevex/tree/newkvstore

These still have some known performance regressions I'm tracking down.

abhinavdangeti added a commit that referenced this issue Jun 6, 2024
Brings in:
* 8d27d20 Rahul Rampure | MB-62167: Fix windows crash (#246)
abhinavdangeti added a commit that referenced this issue Jun 6, 2024
Brings in:
* 8d27d20 Rahul Rampure | MB-62167: Fix windows crash (#246)
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

No branches or pull requests

2 participants