jugglingdb icon indicating copy to clipboard operation
jugglingdb copied to clipboard

updateAttributes on jugglingdb-redis breaks index for non-string fields

Open ragamufin opened this issue 11 years ago • 14 comments

Thank you for all your hard work. I ran into an issue with the jugglingdb-redis adapter. I have several fields that are defined such as:

status: { type: Number, default: 1, index: true }

When I create an instance of the object with that field it saves fine and I can see in the store that there is an index created. However if I call updateAttributes on the same instance later the index actually disappears. I've debugged this and traced it back to the following code: redis.js Line291. It compares the value being saved with the previous value and if they are different it adds the new one and removes the old one. The problem is it uses a !== for comparison so even though you could be passing the same value it will be removed if it is a number, or date or anything but a string. All keys and values in prevData are strings because they come from the hash that was retrieved with a hgetall at line 260. So when you try to do a findOne later on status it fails because it was removed from the index.

Let me know if this makes sense. I'd love to have this working so that I can use numbers, booleans, etc. and still search on them even after updating the records.

ragamufin avatar Dec 13 '13 17:12 ragamufin

Hey @ragamufin it all makes sense. Probably converting to string before comparison should solve the issue. But before you continue with this adapter I should say, that this code was not updated for a long time, since we switched to redis-hq adapter, which uses GET + JSON instead of HMGET to store object and has a lot of improvements on indexing and other useful features. So, if JSON + GET works for your application - it is better to switch. Thanks!

1602 avatar Dec 17 '13 13:12 1602

Thanks for responding. I didn't know about redis-hq. The jugglingdb page doesn't seem to mention it at all. I just took a look at it now and the setup at the top of the page is the same as that for jugglingdb-redis. Trying to install jugglingdb-redis-hq myself with npm gave me a python error... I'm a bit lost here but I'd like to get it to work. Can you let me know or point me to some installation instructions for setting up jugglingdb with the redis-hq adapter?

ragamufin avatar Dec 18 '13 02:12 ragamufin

just as usual: npm install jugglingdb-redis-hq what error are you getting?

On Wed, Dec 18, 2013 at 2:46 AM, ragamufin [email protected] wrote:

Thanks for responding. I didn't know about redis-hq. The jugglingdb page doesn't seem to mention it at all. I just took a look at it now and the setup at the top of the page is the same as that for jugglingdb-redis. Trying to install jugglingdb-redis-hq myself with npm gave me a python error... I'm a bit lost here but I'd like to get it to work. Can you let me know or point me to some installation instructions for setting up jugglingdb with the redis-hq adapter?

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/349#issuecomment-30812029 .

anatoliychakkaev avatar Dec 18 '13 09:12 anatoliychakkaev

I'm getting an error during the install of jugglingdb-redis-hq when it attempts to install hiredis. The error is: gyp ERR! stack Error: C:\WINDOWS\Microsoft.NET\Framework\v4.0.30319\msbuild.exe failed with exit code: 1. I've found some leads as to how to fix it that I've tried but with no luck. I am trying to follow this one now: https://github.com/TooTallNate/node-gyp/wiki/Visual-Studio-2010-Setup in hopes of fixing it. I will let you know if I get it working tomorrow when I'm back to work. Thanks.

ragamufin avatar Dec 18 '13 09:12 ragamufin

As far as I remember there's way around hiredis installation for windows, but I would recommend you to use linux virtual box. Alternatively you can just remove this dependency (it will use js parser for redis then, but it has some issues).

On Wed, Dec 18, 2013 at 9:37 AM, ragamufin [email protected] wrote:

I'm getting an error during the install of jugglingdb-redis-hq when it attempts to install hiredis. The error is: gyp ERR! stack Error: C:\WINDOWS\Microsoft.NET\Framework\v4.0.30319\msbuild.exe failed with exit code: 1. I've found some leads as to how to fix it that I've tried but with no luck. I am trying to follow this one now: https://github.com/TooTallNate/node-gyp/wiki/Visual-Studio-2010-Setup in hopes of fixing it. I will let you know if I get it working tomorrow when I'm back to work. Thanks.

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/349#issuecomment-30828036 .

Thanks, Anatoliy Chakkaev

anatoliychakkaev avatar Dec 18 '13 09:12 anatoliychakkaev

This is all new to me so I'm not sure of the pros and cons of using the js parser versus hiredis. I have Mac at home so I assume I'll have no problems there but my app will need to work on both, actually more so on Windows so I will have to figure this out. Could you tell me more about what hiredis does and what the repercussions of turning it off are?

ragamufin avatar Dec 18 '13 09:12 ragamufin

It's for performance: https://github.com/mranney/node_redis#performance

In addition I can say that js parser has issues in production use.

On Wed, Dec 18, 2013 at 9:55 AM, ragamufin [email protected] wrote:

This is all new to me so I'm not sure of the pros and cons of using the js parser versus hiredis. I have Mac at home so I assume I'll have no problems there but my app will need to work on both, actually more so on Windows so I will have to figure this out. Could you tell me more about what hiredis does and what the repercussions of turning it off are?

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/349#issuecomment-30829041 .

Thanks, Anatoliy Chakkaev

anatoliychakkaev avatar Dec 18 '13 09:12 anatoliychakkaev

Hi again. I've been struggling with this the past few days and I've finally made some progress. I installed hiredis-node and after that I was able to successfully install jugglingdb-redis-hq. However on start up of my app I get an error. I'm defining my schema like this:

new Schema('redis-hq', { port: 6379 });

And the error I'm getting is:

...\node_modules\jugglingdb-redis-hq\node_modules\redis\index.js:504 throw callback_err; ^ Error: zrange lua script is not loaded at RedisHQ.all ...\node_modules\jugglingdb-redis-hq\lib\adapter.js:839:19)

I saw here: https://github.com/jugglingdb/redis-hq-adapter/issues/8 that this error is fixed so I'm assuming I still don't have my setup done correctly. Can you advise? Thanks again for all your help so far.

ragamufin avatar Dec 20 '13 07:12 ragamufin

Could you please post whole example to run?

On Fri, Dec 20, 2013 at 7:44 AM, ragamufin [email protected] wrote:

Hi again. I've been struggling with this the past few days and I've finally made some progress. I installed hiredis-node and after that I was able to successfully install jugglingdb-redis-hq. However on start up of my app I get an error. I'm defining my schema like this:

new Schema(schemaOptions.adapter, { adapter :'redis-hq' ,adapterOptions :{ port: 6379 } });

And the error I'm getting is:

...\node_modules\jugglingdb-redis-hq\node_modules\redis\index.js:504 throw callback_err; ^ Error: zrange lua script is not loaded at RedisHQ.all ...\node_modules\jugglingdb-redis-hq\lib\adapter.js:839:19)

I saw here: jugglingdb/redis-hq-adapter#8https://github.com/jugglingdb/redis-hq-adapter/issues/8that this error is fixed so I'm assuming I still don't have my setup done correctly. Can you advise? Thanks again for all your help so far.

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/349#issuecomment-30993819 .

Thanks, Anatoliy Chakkaev

anatoliychakkaev avatar Dec 20 '13 09:12 anatoliychakkaev

Sure, no problem. I made a simple example to test. It works on my mac but not on my Windows machine.

var Schema  = require('jugglingdb').Schema

var curr_schema = new Schema('redis-hq', { port: 6379 });

console.log('Starting...');

var Page = curr_schema.define(
    'Page'
    ,{
        identifier: { type: String, length: 255, index: true }
    }
);

// create/update the schema
curr_schema.isActual(function(err, actual) {
    if (!actual) {
        curr_schema.autoupdate(function (err) {
            if(err) {
                console.log('Error during autoupdate');
                console.log(err);
            }
        });
    }
});

Page.all(function(err, pages) {
    if (err) {
        console.log('Error:', err);
    } else if(pages){
        console.log('pages', pages);
    } else {
        console.log('No pages found');
    }

    console.log('Ready');
});

The error is the same as above and is not triggered until I do Page.all

ragamufin avatar Dec 21 '13 01:12 ragamufin

And the error is:

Error: zrange lua script is not loaded
    at RedisHQ.all (...\node_modules\jugglingdb-redis-hq\lib\adapter.js:839:19)
    at Function.all (...\node_modules\jugglingdb\lib\model.js:434:25)
    at Schema.<anonymous> (...\node_modules\jugglingdb\lib\model.js:297:16)
    at Schema.g (events.js:180:16)
    at Schema.EventEmitter.emit (events.js:92:17)
    at Schema.<anonymous> (...\node_modules\jugglingdb\lib\schema.js:121:14)
    at Command.callback (...\node_modules\jugglingdb-redis-hq\lib\index.js:84:17)
    at RedisClient.return_error (...\node_modules\jugglingdb-redis-hq\node_modules\redis\index.js:500:25)
    at ReplyParser.<anonymous> (...\node_modules\jugglingdb-redis-hq\node_modules\redis\index.js:260:14)
    at ReplyParser.EventEmitter.emit (events.js:95:17)

ragamufin avatar Dec 21 '13 01:12 ragamufin

Redis is schemaless, so it is not necessary to call autoupdate. If code works on mac, that means something wrong with windows support in any part of stack. If you can debug it and fix problem, I guess windows users would be thankful.

On Sat, Dec 21, 2013 at 1:49 AM, ragamufin [email protected] wrote:

And the error is:

Error: zrange lua script is not loaded at RedisHQ.all (...\node_modules\jugglingdb-redis-hq\lib\adapter.js:839:19) at Function.all (...\node_modules\jugglingdb\lib\model.js:434:25) at Schema. (...\node_modules\jugglingdb\lib\model.js:297:16) at Schema.g (events.js:180:16) at Schema.EventEmitter.emit (events.js:92:17) at Schema. (...\node_modules\jugglingdb\lib\schema.js:121:14) at Command.callback (...\node_modules\jugglingdb-redis-hq\lib\index.js:84:17) at RedisClient.return_error (...\node_modules\jugglingdb-redis-hq\node_modules\redis\index.js:500:25) at ReplyParser. (...\node_modules\jugglingdb-redis-hq\node_modules\redis\index.js:260:14) at ReplyParser.EventEmitter.emit (events.js:95:17)

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/349#issuecomment-31054011 .

Thanks, Anatoliy Chakkaev

anatoliychakkaev avatar Dec 23 '13 13:12 anatoliychakkaev

Ok, I'm up and running. It took days of debugging and fighting wild animals in the jungle but I finally figured it out. The problem comes up at: https://github.com/jugglingdb/redis-hq-adapter/blob/master/lib/zrange-mget.lua#L8.

It seems Lua scripting (whatever that is) was added to redis in version 2.6.17 as stated in the notes at:http://redis.io/download. The general port of redis to windows is 2.4 which is what I had so that particular line was throwing an error. So an upgrade was in order. I followed the instructions here: http://stackoverflow.com/questions/6476945/how-do-i-run-redis-on-windows/20200022#20200022 and I was able to get things running and can now continue with app development.

Thanks again for your help on this.

ragamufin avatar Dec 25 '13 07:12 ragamufin

Also just to add to this, the indexes behave properly under redis-hq and the original problem I was having is gone under this adapter. Thanks!

ragamufin avatar Dec 25 '13 09:12 ragamufin