drachtio / drachtio-siprec-recording-server

SIPREC recording server based on drachtio and rtpengine
MIT License
81 stars 32 forks source link

Possible to add TTL on keys into Redis #14

Open Mickaelh51 opened 5 years ago

Mickaelh51 commented 5 years ago

Hi, I use your dev with freeswitch mode. I want to add TTL to delete redis keys or delete it when I receive BYE. But add TTL is simplest than delete key with BYE no ?

I'm not sure, but I think the redis data is used before the SIPREC 200OK generation. After that, I can delete key into redis....

thanks in advance

davehorton commented 5 years ago

yes, I agree that we should use a TTS to expire redis keys, but that is in fact what I am doing here:

https://github.com/davehorton/drachtio-siprec-recording-server/blob/95a6e710d71d5eba373155821effea7921649f1f/lib/freeswitch-call-handler.js#L111

I set a 10 second expires value.

Are you seeing keys not getting cleared from redis?

Mickaelh51 commented 5 years ago

I knew it ;) but no, all keys stay in redis... I don't understand why... Do you have an idea please ? thanks in advance

davehorton commented 5 years ago

What version of redis server are you running? Per this:

https://redis.io/commands/set

it seems like that option was added in Redis 2.6.12.

you could always try changing the code to do it this way:

client.multi(()
   .set(opts.sessionId, opts.sdp2)
   .expire(opts.sessionId, 10)
   .exec((err, replies) => ..
Mickaelh51 commented 5 years ago

Hi, I use this redis version (not in docker), installed from debian package:

Redis server v=3.2.6 sha=00000000:0 malloc=jemalloc-3.6.0 bits=64 build=c9ca860b301a190d

I tested but I have the same issue :(

the code:

function storeUnusedSdp(opts) {
  return new Promise((resolve) => {
    console.log(`sessionId: ${opts.xcid} / sdp: ${opts.sdp2}`);
    client.multi()
      .set(opts.xcid, opts.sdp2)
      .expire(opts.xcid, 10)
      .exec((err, replies) => {
        if (err) throw err;
        resolve(opts) ;
    }) ;
  });
}

I can see my key but the TTL is -1

127.0.0.1:6379> TTL 201-1560495744.2171985
(integer) -1

I can force a TTL with cli and it woks:

127.0.0.1:6379> EXPIRE 201-1560495744.2171985 10
(integer) 1
127.0.0.1:6379> TTL 201-1560495744.2171985
(integer) 7

After that I downloaded and compiled the last redis server (http://redis-5.0.5.tar.gz/) No changes :( I found this: https://github.com/NodeRedis/node_redis/issues/1000

Other test:

I tested with:

client.setex(opts.xcid, 10, opts.sdp2);

same issue.

But I created other simple script with the same data, and setex works like a charm...

I'm sorry but I don't know what to try.. :(

Mickaelh51 commented 5 years ago

After lot of things, I found. The first 'set' into storeUnusedSdp is set with TTL, but the code rewrite this data into (handleLeg2SiprecInvite -> exchangeSdp) without TTL... So all registers can't be deleted ... I add TTL in exchangeSdp and it works like a charm (but I use setex) ;)


function storeUnusedSdp(opts) {
  console.log(opts)
  return new Promise((resolve) => {
    if (!opts.xcid) { 
      console.log(`No xcid`);
      return opts.res.send(500, {
        headers: {
          'X-Reason': 'No X-cid header'
        }
      }
        );
    }
    console.log(`sessionId: ${opts.xcid} / sdp: ${opts.sdp2}`);
    client.setex(opts.xcid, 10, opts.sdp2, (err, reply) => {
      if (err) throw err;
      resolve(opts) ;
    }) ;
  });
}
-------------
function exchangeSdp(sessionId, sdp) {
  return new Promise((resolve, reject) => {
    client.multi()
      .get(sessionId)
      .setex(sessionId, 10, sdp)
      .exec((err, replies) => {
        if (err) return reject(err);
        resolve(replies[0]);
      });
  });
}
 ```

But I don't understand why you add TTL in the first set (to store unused sdp), and there is no TTL when you use it with the leg2 (in exchangeSdp)