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

Select calls cannot be made in parallel #25

Open
richorama opened this issue Jan 2, 2014 · 5 comments
Open

Select calls cannot be made in parallel #25

richorama opened this issue Jan 2, 2014 · 5 comments

Comments

@richorama
Copy link
Collaborator

The results array is global to the module. if a client makes two simultaneous selects, one select return with also include results from the other select.

I suggest scoping results to a function.

@tracend
Copy link
Collaborator

tracend commented Jan 2, 2014

A good point definitely

@rjrodger
Copy link
Owner

yikes, sorry about that guys. I suck at coding.

richorama adding you as a contrib, see https://github.com/rvagg/node-levelup/blob/master/CONTRIBUTING.md

@Reggino
Copy link

Reggino commented Dec 22, 2015

Argh... learned this the hard way... ( I just sent an e-mail to a few hundred people based on incorrect and mixed up query results :-( ) . Maybe we should point out in the main readme.md this code is to be considered highly beta and not ready for production use?

@tracend
Copy link
Collaborator

tracend commented Dec 23, 2015

Personally I include all requests in an async series, so I don't have that problem.

Having said that, if parallel processing is a common use-case I think it can be reasonably easy to support with a queue system that keeps tabs of different requests.

More than any specific feature, I feel the library will benefit by splitting it to more than one file, so there's more abstraction and easier to extend.

@Reggino
Copy link

Reggino commented Dec 23, 2015

Sorry for my previous crude post... it was just not my day.. :-)

I don't think queuing isn't necessary here: aws-sdk is perfectly able to perform simultaneous queries. It should just be implemented properly (and some tests should be added to prevent regression)

The problem is probably related to the globally scoped var results here https://github.com/rjrodger/simpledb/blob/master/lib/simpledb.js#L124 . As suggested by @richorama that should be scoped to prevent these kind of issues. I agree with @tracend that restructuring the code will probably fix that along the way.

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

4 participants