On 2013/8/14 13:10, Willy Tarreau wrote:
Hi Godbach,
On Wed, Aug 14, 2013 at 10:20:10AM +0800, Godbach wrote:
I have done a new test with this patch, it works well now.
Thanks for testing.
Yes, just do the same test as last node for other nodes to be removed. I
had tried to fixed it just by testing skip_entry but forgetten to test
si->applet.ctx.table.entry->ref_cnt. And the test condition should have
been found for the last node.
It was not obvious for me either, I found it only by single stepping in gdb !
There is another issue I want to make sure. There are nodes to be
deleted even in 'show table' action if expired as below:
eb = ebmb_next(&si->applet.ctx.table.entry->key);
if (eb) {
struct stksess *old = si->applet.ctx.table.entry;
si->applet.ctx.table.entry = ebmb_entry(eb, struct stksess, key);
if (show)
stksess_kill_if_expired(&si->applet.ctx.table.proxy->table, old);
else if (!skip_entry && !si->applet.ctx.table.entry->ref_cnt)
stksess_kill(&si->applet.ctx.table.proxy->table, old);
si->applet.ctx.table.entry->ref_cnt++;
break;
}
If the expired nodes are not removed here, they can still be removed in
expiration task by calling process_table_expire(). So the idea to remove
expired nodes in 'show table' action can make process_table_expire() do
less work.
I've seen this as well and had a hard time reminding why it was done
this way. I was sure it was needed but the cause was not obvious to me.
IIRC, the reason was that we want "show table" to report valid entry
counts, so if we don't kill the entries ourselves and there is low
activity, nothing else will kill them fast enough to have valid counts.
And as you say, it also reduces the work of process_table_expire(),
eventhough this is a very minor benefit since we're not supposed to
be dumping the stats all the day along :-)
Best regards,
Willy
Hi Willy,
Thank you for your replying.
--
Best Regards,
Godbach
