7

I'm following a tutorial and I created a cache.js file that takes the mongoose query and JSON.stringifies it into the key for the values being returned by that query. The goal is to cache that and then append .cache() inside app.js where the mongoose.find() is

Currenty if the cache is empty, I have it do a GET from the DB and then store it in the cache. I have a

console.log("CACHE VALUE #2");
console.log(cacheValue1);

that ensures that the data is getting stored and outputs the data successfully. This line works. But with this line,

console.log("CACHE VALUE #1");
console.log(cacheValue);

the cacheValue is null.

Why is that?

It stores at the bottom the value and the key never changes so I don't understand why it won't return the data instead of null.

So Cache Value #1 is always null and Cache Value #2 has the correct data.

CONSOLE OUTPUT:

GRABBING FROM DB
CLIENT CONNECTION STATUS: true
Setting CACHE to True
ABOUT TO RUN A QUERY
{"$and":[{"auctionType":{"$eq":"publicAuction"}},{"auctionEndDateTime":{"$gte":1582903244869}},{"blacklistGroup":{"$ne":"5e52cca7180a7605ac94648f"}},{"startTime":{"$lte":1582903244869}}],"collection":"listings"}
CACHE VALUE #1
null
CACHE VALUE #2
(THIS IS WHERE ALL MY DATA SHOWS UP)
const mongoose = require('mongoose');
const redis = require('redis');
const util = require('util');
var env = require("dotenv").config({ path: './.env' });

const client = redis.createClient(6380, process.env.REDISCACHEHOSTNAME + '.redis.cache.windows.net', {
  auth_pass: process.env.REDISCACHEKEY,
  tls: { servername: process.env.REDISCACHEHOSTNAME + '.redis.cache.windows.net' }
});


client.get = util.promisify(client.get);


const exec = mongoose.Query.prototype.exec;

mongoose.Query.prototype.cache = function () {
  this.useCache = true;
  console.log("Setting CACHE to True")
  return this;
}

mongoose.Query
  .prototype.exec = async function () {
    if (!this.useCache) {
      console.log("GRABBING FROM DB")
      console.log("CLIENT CONNECTION STATUS: " + client.connected);

      return exec.apply(this, arguments);
    }

    console.log("ABOUT TO RUN A QUERY")
    const key = JSON.stringify(Object.assign({}, this.getQuery(), {
      collection: this.mongooseCollection.name
    }));


    //See if we have a value for 'key' in redis
    console.log(key);
    const cacheValue = await client.get(key);
    console.log("CACHE VALUE #1");
    console.log(cacheValue);
    //If we do, return that
    if (cacheValue) {
      console.log("cacheValue IS TRUE");
      const doc = JSON.parse(cacheValue);
      return Array.isArray(doc)
        ? doc.map(d => new this.model(d))
        : new this.model(doc);
    }

    //Otherwise, issue the query and store the result in redis
    const result = await exec.apply(this, arguments);

    let redisData = JSON.stringify(result);
    //stores the mongoose query result in redis



    await client.set(key, JSON.stringify(redisData)), function (err) {
      console.error(err);

    }
    const cacheValue1 = await client.get(key);
    console.log("CACHE VALUE #2");
    console.log(cacheValue1);




    return result;
  }


user6680
  • 79
  • 6
  • 34
  • 78
  • Are you serving your results with some sort of web framework ( express, koa, restify) if yes it will be much easier to implement with some sort of middleware – C.Gochev Feb 29 '20 at 14:55
  • I'm using Azure Redis with mean stack so yes express as well. I feel like I'm really close to getting it working. The code is called with```.cache()``` like this https://pastebin.com/xW1Lzr82 – user6680 Feb 29 '20 at 17:36
  • Are you certain that the query doesn't change at all between subsequent runs? This code seems fine, beyond the fact your key is very complex (you could hash the object and use the hash as a key instead btw). Your key seems to contains several different timestamps, are you certain these don't change between queries? I would log the query between requests and make certain they haven't changed. – Jon Church Feb 29 '20 at 19:41

2 Answers2

1

Based on the pastebin you linked, your queries are using Date.now() for their values. This means that each time a query is run, you have different values for the timestamp.

Because your keys are the actual query, and the query has dynamic values based on Date.now(), your keys will never be the same, which is why you can't find them in the cache later, each query is generating a unique key because of the dynamic values of Date.now().

Jon Church
  • 2,320
  • 13
  • 23
  • I understand what you're saying and it makes sense. If I output ```console.log(key)```, it outputs the date at that moment in time. This kind of complicates things I guess. Do you have any ideas on how to make everything play nicely together? – user6680 Feb 29 '20 at 20:27
  • You'll have to change either the way you generate keys so they are reproducible, or change how you query for the key. Redis has a lot more commands than basic get and set, but I think the simplest thing to do is fix the keys. You could alter the timestamps (after cloning the query structure) before they're persisted as keys so they're rounded up to the nearest minute or so, but I'm not sure that's the best approach. – Jon Church Mar 01 '20 at 00:39
  • I've been playing with a couple ideas in my head. One where I use regex to replace unix timestamp after evaluating it to true or false, but I'm not sure if that would work evaluating date beforehand and replacing unix timestamp with true/false. I was gonna add certain strategies for caching like create an auction and have it start in 5 minutes and have the cache clear every 4 mins. Also had an idea with replacing timestamp with some sort of tag. Still brainstorming lol – user6680 Mar 01 '20 at 18:57
  • I think you have to ask yourself 2 important questions here about this particular auction query to see if it is worth caching. 1) How often does this data change? 2) Will stale data hurt? Specifically for 1, can a start and end time change? You could filter expired auctions clientside if you're sure the end time won't change. But if your data changes frequently and this query is used somewhere that stale data is unacceptable, then this might not be a good candidate for caching at all. – Jon Church Mar 03 '20 at 19:53
  • It's important to me to try and cache it to some capacity, which is why I will expire the cache every 4 mins. If auctions start in 5 mins then the auctions will always appear in 5 mins and it would save me a bunch of GET requests. I'm using cosmosdb and it ain't cheap. This is just me as a cost forecast https://postimg.cc/DW367ZCn, so if I can offset some it to where it only has a GET every 4 mins instead of 100/1000s of people clicking, then every little bit would count. If I just get rid of the unix timestamps than I think it will still be unique enough to be a key. – user6680 Mar 03 '20 at 20:34
0

I reproduced your code within a minimal example and I got it working as you wanted.It serves the response from the redis cache after the first request. I just logged values and found some small mistakes in your code,you will easily spot them(calling this.model with the doc, fix the set and remove the last get for the redis client). I don't think that this is the best way to cache responses because you set a variable to true in every request and also you reach to mongo and mongoose before you use cache.With a middleware all this can be prevented and you will take away some more ms but still its not the worst way either so here is my minimal working example.

const http = require('http')
const mongoose = require('mongoose')
const redis = require('redis')
const port = 3000;
const util = require('util');

const client = redis.createClient()
client.get = util.promisify(client.get);
mongoose.connect('mongodb://localhost:27017/testdb', {useNewUrlParser: true});
const exec = mongoose.Query.prototype.exec;
  var Schema = mongoose.Schema;

  var testSchema = new Schema({
    testfield:  String, // String is shorthand for {type: String}
  });


 var Test = mongoose.model('Test', testSchema);

mongoose.Query.prototype.cache = function() {
  this.useCache = true;
  console.log("Setting CACHE to True")
  return this;
}

mongoose.Query
  .prototype.exec = async function () {
    if (!this.useCache) {
      console.log("GRABBING FROM DB")
      console.log("CLIENT CONNECTION STATUS: " + client.connected);

      return exec.apply(this, arguments);
    }

    console.log("ABOUT TO RUN A QUERY")
      console.log("Query ==", this.getQuery())
      console.log("Collection == ", this.mongooseCollection.name);
    const key = JSON.stringify(Object.assign({}, this.getQuery(), {
      collection: this.mongooseCollection.name
    }));


    //See if we have a value for 'key' in redis
    console.log("KEY FROM QUERY AND COLLECTION",key);
    const cacheValue = await client.get(key);
    console.log("CACHE VALUE #1");
    console.log(cacheValue);
    //If we do, return that
    if (cacheValue) {
      console.log("cacheValue IS TRUE");
      const doc = JSON.parse(cacheValue);
        console.log("DOC == ",doc);
      return Array.isArray(doc)
        ? doc.map(d => d)
        : doc
           // return exec( Array.isArray(doc)
       // ? doc.map(d => new this.model(d))
        //: new this.model(doc))
    }

    //Otherwise, issue the query and store the result in redis
    const result = await exec.apply(this, arguments);
   // console.log("EXEC === ", exec);
   // console.log("result from query == ", result);
    let redisData = JSON.stringify(result);
    //stores the mongoose query result in redis

    console.log("REDis data ===", redisData);

    await client.set(key, redisData, function (err) {
      console.error(err);

    })




    return result;
  }

const server = http.createServer(function(req, res) {
    if(req.url === '/'){
        Test.find({}).cache().exec().then(function( docs) {
          console.log("DOCS in response == ", docs);
          res.writeHead(200, { 'Content-Type': 'text/plain' });
          res.end(JSON.stringify(docs))
        })
    }

})


server.listen(port, function() {

    console.log(`Server listening on port ${port}`)
 })
C.Gochev
  • 1,837
  • 11
  • 21
  • It's not a problem with their code, it's a problem with particular queries they're using as keys. In your example the query is static, in the pastebin they posted with example queries you'll see that their query has a dynamic `Date.now()` component. Your solution will still have the same problem they're experiencing when used with their queries. – Jon Church Mar 01 '20 at 00:41
  • yes, you are right. saw the pastebin after I posted the answer. can't use Date.now() as a key but there is a solution for this 100% – C.Gochev Mar 01 '20 at 09:52
  • In order to solve this, I believe the easiest thing to do is get the Date.now value when declared and assign it to a variable and pass it to cache.js then I can do a string replace and replace the date with ```""```. But I can't figure out how to pass the value to ```mongoose.Query .prototype.exec = async function () {``` from app.js to cache.js https://pastebin.com/MH3nFx5r – user6680 Mar 04 '20 at 20:05