Re: Attempting to clear entries in stick table based on server id, results in all entries being dropped.

2013-08-14 Thread Godbach

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



Re: Attempting to clear entries in stick table based on server id, results in all entries being dropped.

2013-08-14 Thread Mark Brooks
Thankyou Willy, ive also tested the patch and it works fine.

Thanks again

Mark

On 14 August 2013 07:17, Godbach nylzhao...@gmail.com wrote:
 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



Re: Attempting to clear entries in stick table based on server id, results in all entries being dropped.

2013-08-14 Thread Willy Tarreau
On Wed, Aug 14, 2013 at 11:12:04AM +0100, Mark Brooks wrote:
 Thankyou Willy, ive also tested the patch and it works fine.

Thanks for your feedback Mark!

Willy




Re: Attempting to clear entries in stick table based on server id, results in all entries being dropped.

2013-08-13 Thread Willy Tarreau
Hi guys,

On Thu, Aug 08, 2013 at 08:48:53PM +0800, Godbach wrote:
 On 2013/8/8 18:50, Mark Brooks wrote:
 The issue I am seeing is that using the dev version of HAProxy
 1.5-dev19 git commit id 00f0084752eab236af80e61291d672e835790cff
 
 I have a source IP stick table and im trying to drop specific entries
 from it but its resulting in the whole table being dropped each time.

(...)

I got it, it's a stupid fix for a previous bug that was killing a bit
too much this time.

Here's the fix.

Best regards,
Willy

From 33fba6f78f2e9e9f1274bde10ac1cd86f2804d64 Mon Sep 17 00:00:00 2001
From: Willy Tarreau w...@1wt.eu
Date: Tue, 13 Aug 2013 16:44:40 +0200
Subject: BUG/MINOR: cli: clear table must not kill entries that don't match
 condition

Mark Brooks reported the following issue :

My table looks like this -

  0x24a8294: key=192.168.136.10 use=0 exp=1761492 server_id=3
  0x24a8344: key=192.168.136.11 use=0 exp=1761506 server_id=2
  0x24a83f4: key=192.168.136.12 use=0 exp=1761520 server_id=3
  0x24a84a4: key=192.168.136.13 use=0 exp=1761534 server_id=2
  0x24a8554: key=192.168.136.14 use=0 exp=1761548 server_id=3
  0x24a8604: key=192.168.136.15 use=0 exp=1761563 server_id=2
  0x24a86b4: key=192.168.136.16 use=0 exp=1761580 server_id=3
  0x24a8764: key=192.168.136.17 use=0 exp=1761592 server_id=2
  0x24a8814: key=192.168.136.18 use=0 exp=1761607 server_id=3
  0x24a88c4: key=192.168.136.19 use=0 exp=1761622 server_id=2
  0x24a8974: key=192.168.136.20 use=0 exp=1761636 server_id=3
  0x24a8a24: key=192.168.136.21 use=0 exp=1761649 server_id=2

im running the command -

  socat unix-connect:/var/run/haproxy.stat stdio  'clear table VIP_Name-2 
data.server_id eq 2'

Id assume that the entries with server_id = 2 would be removed but its
removing everything each time.

The cause of the issue is a missing test for skip_entry when deciding
whether to clear the key or not. The test was present when only the
last node is to be removed, so removing only the first node from a
list of two always did the right thing, explaining why it remained
unnoticed in basic unit tests.

The bug was introduced by commit 8fa52f4e which attempted to fix a
previous issue with this feature where only the last node was removed.

This bug is 1.5-specific and does not require any backport.
---
 src/dumpstats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dumpstats.c b/src/dumpstats.c
index 46066b5..8707e22 100644
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -4170,7 +4170,7 @@ static int stats_table_request(struct stream_interface 
*si, int action)
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
+   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;
-- 
1.7.12.2.21.g234cd45.dirty



Re: Attempting to clear entries in stick table based on server id, results in all entries being dropped.

2013-08-13 Thread Godbach

On 2013/8/13 22:54, Willy Tarreau wrote:

Hi guys,

On Thu, Aug 08, 2013 at 08:48:53PM +0800, Godbach wrote:

On 2013/8/8 18:50, Mark Brooks wrote:

The issue I am seeing is that using the dev version of HAProxy
1.5-dev19 git commit id 00f0084752eab236af80e61291d672e835790cff

I have a source IP stick table and im trying to drop specific entries

from it but its resulting in the whole table being dropped each time.


(...)

I got it, it's a stupid fix for a previous bug that was killing a bit
too much this time.

Here's the fix.

Best regards,
Willy



Hi Willy,

I have done a new test with this patch, it works well now.

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.


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.


--
Best Regards,
Godbach



Re: Attempting to clear entries in stick table based on server id, results in all entries being dropped.

2013-08-13 Thread Willy Tarreau
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




Attempting to clear entries in stick table based on server id, results in all entries being dropped.

2013-08-08 Thread Mark Brooks
The issue I am seeing is that using the dev version of HAProxy
1.5-dev19 git commit id 00f0084752eab236af80e61291d672e835790cff

I have a source IP stick table and im trying to drop specific entries
from it but its resulting in the whole table being dropped each time.

My table looks like this -

0x24a8294: key=192.168.136.10 use=0 exp=1761492 server_id=3
0x24a8344: key=192.168.136.11 use=0 exp=1761506 server_id=2
0x24a83f4: key=192.168.136.12 use=0 exp=1761520 server_id=3
0x24a84a4: key=192.168.136.13 use=0 exp=1761534 server_id=2
0x24a8554: key=192.168.136.14 use=0 exp=1761548 server_id=3
0x24a8604: key=192.168.136.15 use=0 exp=1761563 server_id=2
0x24a86b4: key=192.168.136.16 use=0 exp=1761580 server_id=3
0x24a8764: key=192.168.136.17 use=0 exp=1761592 server_id=2
0x24a8814: key=192.168.136.18 use=0 exp=1761607 server_id=3
0x24a88c4: key=192.168.136.19 use=0 exp=1761622 server_id=2
0x24a8974: key=192.168.136.20 use=0 exp=1761636 server_id=3
0x24a8a24: key=192.168.136.21 use=0 exp=1761649 server_id=2

im running the command -

socat unix-connect:/var/run/haproxy.stat stdio  'clear table
VIP_Name-2 data.server_id eq 2'

Id assume that the entries with server_id = 2 would be removed but its
removing everything each time.


My Configuration file is below -

listen VIP_Name-2
bind 192.168.138.2:80 transparent
mode http
balance leastconn
stick-table type ip size 10240k expire 30m
stick on src
server backup 127.0.0.1:9081 backup  non-stick
option httpclose
option forwardfor
option redispatch
option abortonclose
maxconn 4
log global
option httplog
server RIP_Name 192.168.66.50  weight 100  check port 80  inter 2000
rise 2  fall 3 minconn 0  maxconn 0  on-marked-down shutdown-sessions
server RIP_Name-1 192.168.66.51  weight 100  check port 80  inter 2000
 rise 2  fall 3 minconn 0  maxconn 0  on-marked-down shutdown-sessions

Thanks

Mark



Re: Attempting to clear entries in stick table based on server id, results in all entries being dropped.

2013-08-08 Thread Godbach

On 2013/8/8 18:50, Mark Brooks wrote:

The issue I am seeing is that using the dev version of HAProxy
1.5-dev19 git commit id 00f0084752eab236af80e61291d672e835790cff

I have a source IP stick table and im trying to drop specific entries
from it but its resulting in the whole table being dropped each time.

My table looks like this -

0x24a8294: key=192.168.136.10 use=0 exp=1761492 server_id=3
0x24a8344: key=192.168.136.11 use=0 exp=1761506 server_id=2
0x24a83f4: key=192.168.136.12 use=0 exp=1761520 server_id=3
0x24a84a4: key=192.168.136.13 use=0 exp=1761534 server_id=2
0x24a8554: key=192.168.136.14 use=0 exp=1761548 server_id=3
0x24a8604: key=192.168.136.15 use=0 exp=1761563 server_id=2
0x24a86b4: key=192.168.136.16 use=0 exp=1761580 server_id=3
0x24a8764: key=192.168.136.17 use=0 exp=1761592 server_id=2
0x24a8814: key=192.168.136.18 use=0 exp=1761607 server_id=3
0x24a88c4: key=192.168.136.19 use=0 exp=1761622 server_id=2
0x24a8974: key=192.168.136.20 use=0 exp=1761636 server_id=3
0x24a8a24: key=192.168.136.21 use=0 exp=1761649 server_id=2

im running the command -

socat unix-connect:/var/run/haproxy.stat stdio  'clear table
VIP_Name-2 data.server_id eq 2'

Id assume that the entries with server_id = 2 would be removed but its
removing everything each time.


My Configuration file is below -

listen VIP_Name-2
bind 192.168.138.2:80 transparent
mode http
balance leastconn
stick-table type ip size 10240k expire 30m
stick on src
server backup 127.0.0.1:9081 backup  non-stick
option httpclose
option forwardfor
option redispatch
option abortonclose
maxconn 4
log global
option httplog
server RIP_Name 192.168.66.50  weight 100  check port 80  inter 2000
rise 2  fall 3 minconn 0  maxconn 0  on-marked-down shutdown-sessions
server RIP_Name-1 192.168.66.51  weight 100  check port 80  inter 2000
  rise 2  fall 3 minconn 0  maxconn 0  on-marked-down shutdown-sessions

Thanks

Mark




Hi Mark,

I have got the same problem with you. It may be a bug which can also 
reproduce while showing an specified data type with such command as below:

show table table-name data.server_id id

stats_table_request() in dumpstat.c are called to delete stick entries. 
The main codes are as follow:


 if (show  !skip_entry 
 !stats_dump_table_entry_to_buffer(trash, si, 
si-applet.ctx.table.proxy,

   si-applet.ctx.table.entry))
 return 0;

 si-applet.ctx.table.entry-ref_cnt--;

 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
 stksess_kill(si-applet.ctx.table.proxy-table, old);
 si-applet.ctx.table.entry-ref_cnt++;
 break;
 }

It seems that skip_entry should be checked also while calling 
stksess_kill() since the entry is marked as skipped to be deleted.


I have tried to fix this problem but a pity that I cannot fix it 
completely by now. :-(


--
Best Regards,
Godbach