ca98am79 / connect-dynamodb

DynamoDB session store for Connect
http://ca98am79.github.com/connect-dynamodb/
MIT License
144 stars 66 forks source link

Issue with reaping #27

Closed jacobsimon closed 8 years ago

jacobsimon commented 8 years ago

Not sure why, but reaping does not appear to be working for me, and the number of sessions stored in dynamo is growing seemingly without bound. I'm not using a custom reapInterval so it should be using the 10 min default, but there are sessions that are weeks old in my table. Any help would be appreciated!

wcchoi commented 8 years ago

I faced similar problem a while ago. The reason is that if you have too many session items in a DynamoDB Table, and you would like to scan all the items, a single call to Scan will not return all the items, you have to get the LastEvaluatedKey return value from the Scan call and pass it to the next Scan call. (see http://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_Scan.html)

Judging from the source code for the reap function (https://github.com/ca98am79/connect-dynamodb/blob/master/lib/connect-dynamodb.js#L194), it seems that this library only scan for the first "page" of result, and if none of the items in the first page expires, nothing is deleted, so the items will keep growing.

The fix is rather easy, just modify that part of code to keep calling Scan until LastEvaluatedKey is empty.
PS. You may need to add some delay between calls to prevent blowing up the provision throughput.

vmkcom commented 8 years ago

Same issue, maybe it's not a good idea to Scan all database with every reap cause we can easily reach the Read / Write(Delete) limit.

Maybe we need option for switching of Reap completely for implementing our own reaping process, or override reap function completely

ca98am79 commented 8 years ago

@wcchoi thanks for this - could you do a PR for this?

ca98am79 commented 8 years ago

@vmkcom this makes sense

mblenton commented 8 years ago

I think this is what @wcchoi is talking about:

    this.client.scan(params, function onScan(err, data) {
        if (err) return fn && fn(err);
        destroy.call(this, data, fn);
        if (typeof data.LastEvaluatedKey != "undefined") {
            //console.log("Scanning for more...");
            params.ExclusiveStartKey = data.LastEvaluatedKey;
            this.client.scan(params, onScan);
        }          
    }.bind(this));
ca98am79 commented 8 years ago

fixed with https://github.com/ca98am79/connect-dynamodb/pull/29