No call count

[mathbruyen]

There used to be a counter for the number of calls per period (s_calls and s_avgCalls), but it seems that this count is not used anymore.

[lloyd]

very good catch. I want to do some testing before merging, as I think using s_avgCalls to factor into the code that determines how many calls to block might address this issue: https://hacks.mozilla.org/2013/01/building-a-node-js-server-that-wont-melt-a-node-js-holiday-season-part-5/comment-page-1/#comment-1951172

[mathbruyen]

Funny enough the graph in the issue looks like TCP congestion avoidance.

I would not include call count in the algorithm because the number of calls a server can handle heavily depends on itself and the application. I would search more around reducing the decay factor (to reduce metric variance, load on a server usually does not change too much within seconds) and adding a coefficient in the computation of pctToBlock.

Anyway it's really a personal opinion and quite off-topic, good luck with the issue!

[ThisIsMissEm]

If you were to do this, you could probably write it as:

Handle<Value> TooBusy(const Arguments& args) {
    // No HandleScope required, because this function allocates no
    // v8 classes that reside on the heap.
    if (s_currentLag > HIGH_WATER_MARK_MS) {
        // probabilistically block requests proportional to how
        // far behind we are.
        double pctToBlock = ((s_currentLag - HIGH_WATER_MARK_MS) /
                             (double) HIGH_WATER_MARK_MS) * 100.0;
        double r = (rand() / (double) RAND_MAX) * 100.0;

        if (r < pctToBlock) return True();
    }
    return False();
}

[mathbruyen]

Right, it depends how you shape your code. @lloyd any syntax preference?

[lloyd]

yeah. so I spent an hour playing around with alternative algorithms that leverage call count and there is no clear win. let's clean up the code for now! merging.