Module: sip-router Branch: master Commit: 3e3ec3e302e25936af1b24a3769a455abc138e08 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=3e3ec3e3...
Author: Juha Heinanen jh@tutpro.com Committer: Juha Heinanen jh@tutpro.com Date: Sun May 9 15:02:11 2010 +0300
modules_k/permissions: fixed reloading of address table - Database handle was not initialized when address table was reloaded. Now it is initialized/closed at each reload. Looks like this has never been tested before. Still don't know why mi_addr_child_init() exists and what it is supposed to do.
---
modules_k/permissions/address.c | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/modules_k/permissions/address.c b/modules_k/permissions/address.c index 469e21a..0dd7b10 100644 --- a/modules_k/permissions/address.c +++ b/modules_k/permissions/address.c @@ -79,6 +79,17 @@ int reload_address_table(void) cols[2] = &mask_col; cols[3] = &port_col; cols[4] = &tag_col; + + if (db_handle) { + LM_ERR("db_handle already exists\n"); + return -1; + } + + db_handle = perm_dbf.init(&db_url); + if (!db_handle) { + LM_ERR("unable to connect database\n"); + return -1; + }
if (perm_dbf.use_table(db_handle, &address_table) < 0) { LM_ERR("failed to use table\n"); @@ -166,6 +177,9 @@ int reload_address_table(void)
perm_dbf.free_result(db_handle, res);
+ perm_dbf.close(db_handle); + db_handle = 0; + *addr_hash_table = new_hash_table; *subnet_table = new_subnet_table;
@@ -208,9 +222,13 @@ int init_addresses(void) if(db_check_table_version(&perm_dbf, db_handle, &address_table, TABLE_VERSION) < 0) { LM_ERR("error during table version check.\n"); perm_dbf.close(db_handle); + db_handle = 0; return -1; }
+ perm_dbf.close(db_handle); + db_handle = 0; + addr_hash_table_1 = new_addr_hash_table(); if (!addr_hash_table_1) return -1;
@@ -245,9 +263,6 @@ int init_addresses(void) goto error; }
- perm_dbf.close(db_handle); - db_handle = 0; - return 0;
error: @@ -275,8 +290,6 @@ error: shm_free(subnet_table); subnet_table = 0; } - perm_dbf.close(db_handle); - db_handle = 0; return -1; }
2010/5/9 Juha Heinanen jh@tutpro.com:
modules_k/permissions: fixed reloading of address table
- Database handle was not initialized when address table was reloaded.
Now it is initialized/closed at each reload. Looks like this has never been tested before. Still don't know why mi_addr_child_init() exists and what it is supposed to do.
Does the same problem exist in Kamailio 1.5.4? if so, will be a backport for it? I already reported a very annoying problem using "address_reload" under heavy traffic load (kamailio gets totally frozen).
Regards.
Iñaki Baz Castillo writes:
- Database handle was not initialized when address table was reloaded.
Now it is initialized/closed at each reload. Looks like this has never been tested before. Still don't know why mi_addr_child_init() exists and what it is supposed to do.
Does the same problem exist in Kamailio 1.5.4? if so, will be a backport for it?
i have not noticed this problem in k 1.5. if you give address_reload command in 1.5, does it work? in 3.0 and above, it printed an error message about mysql not been able to use address table.
I already reported a very annoying problem using "address_reload" under heavy traffic load (kamailio gets totally frozen).
did you have lots of addresses in the table?
-- juha
2010/5/10 Juha Heinanen jh@tutpro.com:
Iñaki Baz Castillo writes:
> > - Database handle was not initialized when address table was reloaded. > > Now it is initialized/closed at each reload. Looks like this has > > never been tested before. Still don't know why mi_addr_child_init() > > exists and what it is supposed to do. > > > Does the same problem exist in Kamailio 1.5.4? if so, will be a > backport for it?
i have not noticed this problem in k 1.5. if you give address_reload command in 1.5, does it work? in 3.0 and above, it printed an error message about mysql not been able to use address table.
It works in 1.5, but at least in 1.5.1 *under heavy traffic* kamailio gets frozen. I couldn't reproduce the problem in my testing environment yet so I don't know if it doesn't occur in 1.5.4.
So I think the commit cannot be related to the issue I mean.
> I already reported a very annoying problem using "address_reload" > under heavy traffic load (kamailio gets totally frozen).
did you have lots of addresses in the table?
No, just 10 entries. But now I'm building a kamailio making extensive usage of address table to identify trunk clients based on the source IP. The adin web interface performs a XMLRPC command to invoke "address_reload" for each new client addition/modification. Could I get into issues if the address table contains lots of entries?
Thanks.
Iñaki Baz Castillo writes:
No, just 10 entries. But now I'm building a kamailio making extensive usage of address table to identify trunk clients based on the source IP. The adin web interface performs a XMLRPC command to invoke "address_reload" for each new client addition/modification. Could I get into issues if the address table contains lots of entries?
currently permissions module does not have a parameter that tells how many records to load at one shot. that should be introduced if you have thousands of records in address table.
-- juha
2010/5/10 Juha Heinanen jh@tutpro.com:
Iñaki Baz Castillo writes:
> No, just 10 entries. But now I'm building a kamailio making extensive > usage of address table to identify trunk clients based on the source > IP. The adin web interface performs a XMLRPC command to invoke > "address_reload" for each new client addition/modification. Could I > get into issues if the address table contains lots of entries?
currently permissions module does not have a parameter that tells how many records to load at one shot. that should be introduced if you have thousands of records in address table.
Hi Juha, I don't fully understand what you mean. Which parameter do you mean? do you say that 1.5.4 permissions module is not suitable for thousands of records. This would be a big handicap in my plattform.
Thanks.
Iñaki Baz Castillo writes:
Hi Juha, I don't fully understand what you mean. Which parameter do you mean? do you say that 1.5.4 permissions module is not suitable for thousands of records. This would be a big handicap in my plattform.
modules that loads lots of records from db usually have a parameter, such as lcr's fetch_rows, that tells how many records to load at one shot. db experts can tell, why this is a good idea and what problems may happen if such a param does not exist. there are no problems with using the addresses once the records have been successfully loaded.
-- juha
2010/5/10 Juha Heinanen jh@tutpro.com:
Iñaki Baz Castillo writes:
> Hi Juha, I don't fully understand what you mean. Which parameter do > you mean? do you say that 1.5.4 permissions module is not suitable for > thousands of records. This would be a big handicap in my plattform.
modules that loads lots of records from db usually have a parameter, such as lcr's fetch_rows, that tells how many records to load at one shot. db experts can tell, why this is a good idea and what problems may happen if such a param does not exist. there are no problems with using the addresses once the records have been successfully loaded.
I understand. And could one of these db experts tell us the exact problem it could occur?:)
Iñaki Baz Castillo wrote:
2010/5/10 Juha Heinanen jh@tutpro.com:
Iñaki Baz Castillo writes:
Hi Juha, I don't fully understand what you mean. Which parameter do you mean? do you say that 1.5.4 permissions module is not suitable for thousands of records. This would be a big handicap in my plattform.
modules that loads lots of records from db usually have a parameter, such as lcr's fetch_rows, that tells how many records to load at one shot. db experts can tell, why this is a good idea and what problems may happen if such a param does not exist. there are no problems with using the addresses once the records have been successfully loaded.
I understand. And could one of these db experts tell us the exact problem it could occur?:)
Out of memory.
In early days, with "fetch" support, you could encounter problems when restarting *ser if there were a lot of entries in location table as all of them had to fit into memory (I private memory, but can't remember the details). So the workaround was to increase memory but that was inefficient as the memory was needed only during startup.
Now with "fetch" support this problem is gone as only the currently fetched rows have to fit into the memory.
regards klaus
2010/5/10 Klaus Darilion klaus.mailinglists@pernau.at:
In early days, with "fetch" support, you could encounter problems when restarting *ser if there were a lot of entries in location table as all of them had to fit into memory (I private memory, but can't remember the details). So the workaround was to increase memory but that was inefficient as the memory was needed only during startup.
Now with "fetch" support this problem is gone as only the currently fetched rows have to fit into the memory.
2010/5/10 Klaus Darilion klaus.mailinglists@pernau.at:
I understand. And could one of these db experts tell us the exact problem it could occur?:)
Out of memory.
In early days, with "fetch" support, you could encounter problems when restarting *ser if there were a lot of entries in location table as all of them had to fit into memory (I private memory, but can't remember the details). So the workaround was to increase memory but that was inefficient as the memory was needed only during startup.
Now with "fetch" support this problem is gone as only the currently fetched rows have to fit into the memory.
Ok, I understand. Then I should take care in my design. I'm using 'address' table to identify each client based on it(s) source IP(s), using 'grp_id' column which points to each client id.
It's expected that the number of clients will be increased terribly, perhaps 1000-2000, so I could have the same number (or more) of entries in 'address' table. Currently I use 12 or 16 MB of PKG_MEM, could I get into problems?
This is very important for me and I would appreciate any help and confirmation about it. If current (1.5.4) version of 'permissions' module is not suitable for this task then I would use a custom table and perform custom DB queries to match the source address to the client it belongs to.
Thanks a lot.
Iñaki Baz Castillo writes:
This is very important for me and I would appreciate any help and confirmation about it. If current (1.5.4) version of 'permissions' module is not suitable for this task then I would use a custom table and perform custom DB queries to match the source address to the client it belongs to.
inaki,
it would not be a big deal to add a fetch_count variable to permissions module. just copy the idea from one of the other modules that already has it.
-- juha
2010/5/11 Juha Heinanen jh@tutpro.com:
Iñaki Baz Castillo writes:
> This is very important for me and I would appreciate any help and > confirmation about it. If current (1.5.4) version of 'permissions' > module is not suitable for this task then I would use a custom table > and perform custom DB queries to match the source address to the > client it belongs to.
inaki,
it would not be a big deal to add a fetch_count variable to permissions module. just copy the idea from one of the other modules that already has it.
ok. I'll try it. Thanks a lot.
On Tuesday 11 May 2010, Iñaki Baz Castillo wrote:
Now with "fetch" support this problem is gone as only the currently fetched rows have to fit into the memory.
Ok, I understand. Then I should take care in my design. I'm using 'address' table to identify each client based on it(s) source IP(s), using 'grp_id' column which points to each client id.
It's expected that the number of clients will be increased terribly, perhaps 1000-2000, so I could have the same number (or more) of entries in 'address' table. Currently I use 12 or 16 MB of PKG_MEM, could I get into problems?
Hi Iñaki,
you could easily test if the number of records is too much for your current PKG_MEM pool setting by experiment. You should be able to observe memory allocation errors in the logs from the module or the database API. If you choose something like 1/2 or 2/3 of the maximum setting, you should be save.
There is also another reason to prefer a partitioned loading mechanism, as it generate less fragmentation in the PKG_MEM pool, so its "easier" for the memory manager to cope with this workload.
Cheers,
Henning
2010/5/11 Henning Westerholt henning.westerholt@1und1.de:
you could easily test if the number of records is too much for your current PKG_MEM pool setting by experiment. You should be able to observe memory allocation errors in the logs from the module or the database API. If you choose something like 1/2 or 2/3 of the maximum setting, you should be save.
Thanks. For now there are no entries in 'address' table (just 10-20 entries). Also PKG_MEM is 16 MB so I expect I will have enough time to add the fetch_count feature to the 'permissions' module :)
There is also another reason to prefer a partitioned loading mechanism, as it generate less fragmentation in the PKG_MEM pool, so its "easier" for the memory manager to cope with this workload.
Nice to know. Thanks a lot.
Hello,
On 5/9/10 2:08 PM, Juha Heinanen wrote:
Module: sip-router Branch: master Commit: 3e3ec3e302e25936af1b24a3769a455abc138e08 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=3e3ec3e3...
Author: Juha Heinanenjh@tutpro.com Committer: Juha Heinanenjh@tutpro.com Date: Sun May 9 15:02:11 2010 +0300
modules_k/permissions: fixed reloading of address table
- Database handle was not initialized when address table was reloaded. Now it is initialized/closed at each reload. Looks like this has never been tested before. Still don't know why mi_addr_child_init() exists and what it is supposed to do.
are you using the command via MI transports or over RPC?
Daniel
modules_k/permissions/address.c | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/modules_k/permissions/address.c b/modules_k/permissions/address.c index 469e21a..0dd7b10 100644 --- a/modules_k/permissions/address.c +++ b/modules_k/permissions/address.c @@ -79,6 +79,17 @@ int reload_address_table(void) cols[2] =&mask_col; cols[3] =&port_col; cols[4] =&tag_col;
if (db_handle) {
LM_ERR("db_handle already exists\n");
return -1;
}
db_handle = perm_dbf.init(&db_url);
if (!db_handle) {
LM_ERR("unable to connect database\n");
return -1;
}
if (perm_dbf.use_table(db_handle,&address_table)< 0) { LM_ERR("failed to use table\n");
@@ -166,6 +177,9 @@ int reload_address_table(void)
perm_dbf.free_result(db_handle, res);
- perm_dbf.close(db_handle);
- db_handle = 0;
*addr_hash_table = new_hash_table; *subnet_table = new_subnet_table;@@ -208,9 +222,13 @@ int init_addresses(void) if(db_check_table_version(&perm_dbf, db_handle,&address_table, TABLE_VERSION)< 0) { LM_ERR("error during table version check.\n"); perm_dbf.close(db_handle);
db_handle = 0;return -1; }
perm_dbf.close(db_handle);
db_handle = 0;
addr_hash_table_1 = new_addr_hash_table(); if (!addr_hash_table_1) return -1;@@ -245,9 +263,6 @@ int init_addresses(void) goto error; }
perm_dbf.close(db_handle);
db_handle = 0;
return 0;error:
@@ -275,8 +290,6 @@ error: shm_free(subnet_table); subnet_table = 0; }
- perm_dbf.close(db_handle);
- db_handle = 0; return -1; }
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
Daniel-Constantin Mierla writes:
modules_k/permissions: fixed reloading of address table
- Database handle was not initialized when address table was reloaded. Now it is initialized/closed at each reload. Looks like this has never been tested before. Still don't know why mi_addr_child_init() exists and what it is supposed to do.
are you using the command via MI transports or over RPC?
i tried with
sercmd mi address_reload
mi_addr_child_init() initializes db_handle, which i think is not necessary now when reload_address_table does the same and checks that the handle is not initialized. perhaps mi_addr_child_init() could be deleted?
-- juha
On 5/10/10 3:23 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
modules_k/permissions: fixed reloading of address table
- Database handle was not initialized when address table was reloaded. Now it is initialized/closed at each reload. Looks like this has never been tested before. Still don't know why mi_addr_child_init() exists and what it is supposed to do.
are you using the command via MI transports or over RPC?
i tried with
sercmd mi address_reload
mi_addr_child_init() initializes db_handle, which i think is not necessary now when reload_address_table does the same and checks that the handle is not initialized. perhaps mi_addr_child_init() could be deleted?
I think the error is that mi_rpc does not initialize the MI process created by ctl module. So you fixed for address reload but there are other modules in same situation. I will look deeper at it and fix it.
Cheers, Daniel
-
Daniel-Constantin Mierla -
Henning Westerholt -
Iñaki Baz Castillo -
Juha Heinanen -
Klaus Darilion