Add max and ttl for reponse maps

[derekcollison]

Add in ability to expire and limit number of response map entries for accounts mapping request/reply where the response is not sent.

Signed-off-by: Derek Collison derek@nats.io

/cc @nats-io/core

[coveralls]

Coverage increased (+0.1%) to 92.097% when pulling 364f3fe on expire into fd3e5fd on master.

[wallyqs]

can use 10 * time.Minute to define duration too

[derekcollison]

Ah yes thx.

[kozlovic]

That's seem wrong: there is a race where adding service will see that pruning is still true and not trigger the go routine. I believe that the decision to return should be mutually exclusive with the decision to start pruning.

[derekcollison]

Many maps can be removed or added by the time this function executes, so can't really make any assumptions.

[kozlovic]

But you see what I meant, right? If services maps are added while this go routine executes and before it returns where it sets pruning to false, then those added ones will not be pruned, until a new one is added that starts the go routine.

[derekcollison]

I believe I coded it such that if we are over the max or any ones are above ttl we will keep pruning. We do not hold the lock exclusively while we are pruning. We only exit when we are below max and we check ttl first.

[kozlovic]

See above, you check that numOver is <= 0 (meaning that we are below the limit) and then you return. There is a time window between that time and when the pruning will be set to false. At the same time, when services are added, we have this code (under lock):

if a.nae > a.maxnae && !a.prunning {
    a.prunning = true
    go a.pruneAutoExpireResponseMaps()
}

My argument is that it is possible that we now go over the limit and do not have a go routine that will do the pruning (since the previous one is about to exit). Granted, as soon as a new addition (over the limit) will be done, then pruning routine will be started.

[derekcollison]

Yes there could be some condition. I felt it was good to not hold the lock the complete time for processing, however I could do that. It may impact request/response latency more than the current method, but probably not too much unless there are a large number of entries. WDYT? Simplify and hold the lock throughout?

[kozlovic]

I think something like this would be enough?
Remove defer at top of function and then:

a.mu.Lock()
numOver := a.nae - a.maxnae
if numOver <= 0 {
    a.prunning = false
    a.mu.Unlock()
    return
}
a.mu.Unlock()

[derekcollison]

That will not handle any ttl expired messages unless we are also over the limit. Although we do not call on that condition yet, I wanted to the code flexible enough to handle. The test for this exercises both max and ttl conditions.

[kozlovic]

@derekcollison Can always iterate later. LGTM.

[derekcollison]

ok will change prunning to pruning on master.