oreporan / wePlayMin

WePlay - the social football managing app
0 stars 1 forks source link

Adding cache to mongoose #11

Closed oreporan closed 8 years ago

oreporan commented 9 years ago

We need to do this, it doesn't make sense that 3-4 times in one request we try to get the user by Id... There is a cache module that looks pretty easy -

https://www.npmjs.com/package/mongoose-cachebox

Yahavw commented 9 years ago

I tried to add mongoose-cachebox to the League model, and then running the test with no local db, and there was no improvements, maybe I didn't use it properly, and the cache didn't take effect. There is more node.js cache solutions we can try.

oreporan commented 9 years ago

I think the only way to properly test it is by creating some test that gets a user two times and then gets the user two times directly from The db or something...and compares times..

On 19 בספט 2015, at 12:45, Yahav notifications@github.com wrote:

I tried to add mongoose-cachebox to the League model, and then running the test with no local db, and there was no improvements, maybe I didn't use it properly, and the cache didn't take effect. There is more node.js cache solutions we can try.

— Reply to this email directly or view it on GitHub.

Yahavw commented 9 years ago

Did so. Added a new test file, for User model, and tried to play more with the mongoose, without any results. this is the test I wrote(didn't push):

/**
 * Test cache performance, should run with remote DB
 */
describe('#DbCache', function() {
    it('Create a user, then get that user by ID 4 time', function(done) {
        var userName = 'getUser';
        // Generate a user 
        var user = test.generateUser(userName);
        test.saveUserToDB(user, function(client1) {
            logger.test('DbCache', 'saved client1: ' + client1._id);

            User.getUserById(client1._id, function(err, client2) {
                logger.test('DbCache', ' client2 ID: ' + client2._id);

                User.getUserById(client2._id, function(err, client3) {
                    logger.test('DbCache', 'user2 found');

                    User.getUserById(client3._id, function(err, client4) {
                        logger.test('DbCache', 'user3 found');

                        expect(client4.name).to.eql(userName);

                        done();
                    });
                });
            });
        });
    });
});
Yahavw commented 9 years ago

Following the mongoose doc, I added this to the User function:

var mongooseCachebox = require('mongoose-cachebox');

//adding mongoose cachebox
var options = {
        cache: true, // start caching 
        ttl: 30 // 30 seconds 
};

mongooseCachebox(mongoose, options);
module.exports.getUserById = function( clientId , callback ) {
  User.findById(clientId, callback)
  .cache()
  .exec(function (err, docs) { /* ... */

      if (err) throw error;

      console.log(docs.ttl) // time left for expiration in ms 
      console.log(docs.stored); // timestamp this query was cached 
      console.log(docs);

    });
};

But got error: Cannot read property 'cache' of undefined

Yahavw commented 9 years ago

Looks like a known issue: https://github.com/cayasso/mongoose-cachebox/issues/1

oreporan commented 9 years ago

fuck them...lets use a different cache

oreporan commented 9 years ago

I think we should use another cache, because I can see this taking a lot of time

Yahavw commented 9 years ago

I will try something else

On Wed, Sep 30, 2015, 03:45 oreporan notifications@github.com wrote:

I think we should use another cache, because I can see this taking a lot of time

— Reply to this email directly or view it on GitHub https://github.com/oreporan/wePlayMin/issues/11#issuecomment-144315675.

Yahavw commented 8 years ago

Node-cache gave very good improvements. Now need to add the cache to all models.

oreporan commented 8 years ago

good don't do the Game schema yet, because I'm going to remove it

Yahavw commented 8 years ago

👍

2015-10-12 17:00 GMT+03:00 oreporan notifications@github.com:

good don't do the Game schema yet, because I'm going to remove it

— Reply to this email directly or view it on GitHub https://github.com/oreporan/wePlayMin/issues/11#issuecomment-147407322.

Yahavw commented 8 years ago

Should we cache the getUserByName and authenticateUser ? Because they exclude the password

Yahavw commented 8 years ago

Need to merge and then commit. Done with User cache, will move to League

Yahavw commented 8 years ago

Done

oreporan commented 8 years ago

:clap: That was fast!

Maybe we can write a log only if it is found in the cache?

Yahavw commented 8 years ago

Yes, I will remove the not found one.

2015-10-22 10:11 GMT+03:00 oreporan notifications@github.com:

[image: :clap:] That was fast!

Maybe we can write a log only if it is found in the cache?

— Reply to this email directly or view it on GitHub https://github.com/oreporan/wePlayMin/issues/11#issuecomment-150128043.

oreporan commented 8 years ago

We don't need this either -

[INFO ][10/27/2015 17:55:23][wpCache.setById] object cached with id: 562e97080f3596b12fd023e2

oreporan commented 8 years ago

[INFO ][10/27/2015 18:7:29][wpCache.getById] object with id: 562e97080f3596b12fd023e2 not found in cache