Hi Alex,
Thanks for the patch so this wont restart all SMSCs ( as I have done in the patch,sent to you earlier :) ). instead we need to give id of newly added SMSC or old SMSC id for deletion, right? Few more queries 1. How to dynamically change few configurations like throttling capacity for particular SMSC? 2. What happens when throttling capacity for particular SMSC exceeded, how the message are routed, do these message will be delivered to other SMSCs? Thanks, Manish Nema On Thu, 23 Jul 2009 15:37:04 +0530 wrote >Hi Alex, > >sorry, too busy to review... > >Thanks, >Alex > >Am 23.07.2009 um 11:54 schrieb Alejandro Guerrieri: > >> Comments? >> -- >> Alejandro Guerrieri >> [email protected] >> >> >> >> On 16/07/2009, at 18:04, Alejandro Guerrieri wrote: >> >>> Ok, here's a new version with the producer lock removed and a new >>> approach to restart the binds. I had to rewrite a good part of it >>> to make it "transactional" and be able to roll-back to the original >>> group list if the new configuration failed at any point. >>> >>> BTW, I'm not sure about "gwlist_destroy(smsc_groups_copy, NULL);" >>> shall I destroy the group items? >>> >>> Please review it and let me know what you think. >>> >>> Regards, >>> -- >>> Alejandro Guerrieri >>> [email protected] >>> >>> >>> >>> On 13/07/2009, at 21:21, Alexander Malysh wrote: >>> >>>> Hi Alex, >>>> >>>> sorry for delay, here some comments: >>>> >>>> gw_rwlock_wrlock(&smsc_list_lock); >>>> + gwlist_add_producer(smsc_list); >>>> >>>> you don't need to add producer to smsc_list because all >>>> synchronization should go via rwlock smsc_list_lock. >>>> The same seems also the case for smsc2_remove_smsc/ >>>> smsc2_add_smsc.... >>>> >>>> + >>>> + /* reload the groups from the config file */ >>>> + if (bb_reload_smsc_groups() != 0) { >>>> + gwlist_remove_producer(smsc_list); >>>> + gw_rwlock_unlock(&smsc_list_lock); >>>> + return -1; >>>> } >>>> >>>> I think it would be better to try to reload config and only then >>>> shutdown/start SMSC links. Because you should at least ensure >>>> that if config file is not readable that you have all old SMSC >>>> links intact. >>>> >>>> + if (conn != NULL) { >>>> + gwlist_append(smsc_list, conn); >>>> + smscconn_start(conn); >>>> + success = 1; >>>> } >>>> >>>> Please fix indentation... >>>> >>>> Thanks, >>>> Alex >>>> >>>> Am 09.07.2009 um 16:04 schrieb Alejandro Guerrieri: >>>> >>>>> Alex, >>>>> >>>>> Please try this one. >>>>> >>>>> It works by first stopping and removing all running instances, >>>>> then it reloads the groups from the config file and tries to add >>>>> and start the instances again. It also fixes a bug on remove-smsc >>>>> (some instances would fail to be removed when sharing the same >>>>> admin-id). >>>>> >>>>> Things to note: >>>>> >>>>> 1. I've tried the scenario you've depicted yesterday and now it >>>>> works as expected (the extra instances are removed). >>>>> 2. You don't need to stop the smsc first to re-start it. Since >>>>> the start process removes the running instances, you can run >>>>> "start-smsc" and it will stop->remove->reload->add->start. >>>>> Perhaps "restart-smsc" or "reload-smsc" would be more appropiate, >>>>> to be honest, though that would probably confuse a few people. >>>>> >>>>> >>>>> Regards, >>>>> -- >>>>> Alejandro Guerrieri >>>>> [email protected] >>>>> >>>>> >>>>> On 09/07/2009, at 9:21, Alexander Malysh wrote: >>>>> >>>>>> >>>>>> Am 09.07.2009 um 02:23 schrieb Alejandro Guerrieri: >>>>>> >>>>>>> Alex, >>>>>>> >>>>>>> I'm fixing it. I'll be back with a new patch tomorrow. >>>>>> >>>>>> Thanks! >>>>>> >>>>>>> >>>>>>> Regards, >>>>>>> -- >>>>>>> Alejandro Guerrieri >>>>>>> [email protected] >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 08/07/2009, at 22:23, Alexander Malysh wrote: >>>>>>> >>>>>>>> Hi again, >>>>>>>> >>>>>>>> Am 08.07.2009 um 18:28 schrieb Alejandro Guerrieri: >>>>>>>> >>>>>>>>> Alex, >>>>>>>>> >>>>>>>>> I've already fixed the warnings (two variables declared but >>>>>>>>> not used). >>>>>>>>> >>>>>>>>> Regarding the example you've given, that's why I've noted >>>>>>>>> that you shouldn't play with the id/admin-id's. >>>>>>>>> >>>>>>>>> Under my understanding, restart is meant to be used when you >>>>>>>>> need to modify some parameters. Modifying the number of binds >>>>>>>>> qualifies for a remove-smsc/add-smsc. >>>>>>>>> >>>>>>>>> On the scenario you depict, if you remove-smsc A and then add- >>>>>>>>> smsc A, it'll do as expected. >>>>>>>>> >>>>>>>>> A possible approach would be to replace restart-smsc for: >>>>>>>>> >>>>>>>>> get lock >>>>>>>>> remove-smsc >>>>>>>>> add-smsc >>>>>>>>> release lock >>>>>>>>> >>>>>>>>> Or just leave it as it is and document it better? >>>>>>>> >>>>>>>> this is bug and bugs should not be documented their should be >>>>>>>> fixed ;) >>>>>>>> Before your patch was applied it was not possible to provoke >>>>>>>> such situation now it's possible and >>>>>>>> easy fixable. I would like to see patch to fix it or I will do >>>>>>>> this myself :) >>>>>>>> >>>>>>>> Should I fix it or you? >>>>>>>> >>>>>>>> I just already see user complains :) >>>>>>>> >>>>>>>>> >>>>>>>>> What do you think? >>>>>>>>> -- >>>>>>>>> Alejandro Guerrieri >>>>>>>>> [email protected] >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 08/07/2009, at 17:40, Alexander Malysh wrote: >>>>>>>>> >>>>>>>>>> Hi Alex, >>>>>>>>>> >>>>>>>>>> sorry I have not seen this before: >>>>>>>>>> gw/bb_smscconn.c: In function smsc2_remove_smsc: >>>>>>>>>> gw/bb_smscconn.c:839: warning: unused variable smscid >>>>>>>>>> gw/bb_smscconn.c: In function smsc2_add_smsc: >>>>>>>>>> gw/bb_smscconn.c:870: warning: unused variable smsc_type >>>>>>>>>> >>>>>>>>>> Please fix these warnings. Could you please test compile at >>>>>>>>>> your devel host with >>>>>>>>>> ./configure --enable-warnings ... >>>>>>>>>> then you will see these :) >>>>>>>>>> >>>>>>>>>> And I think that now, with config reload, smsc2_restart_smsc >>>>>>>>>> function don't work as expected. >>>>>>>>>> There is example: >>>>>>>>>> >>>>>>>>>> group = smsc >>>>>>>>>> smsc-id = A >>>>>>>>>> >>>>>>>>>> group = smsc >>>>>>>>>> smsc-id = A >>>>>>>>>> >>>>>>>>>> group = smsc >>>>>>>>>> smsc-id = A >>>>>>>>>> >>>>>>>>>> -> start bearerbox >>>>>>>>>> >>>>>>>>>> -> reconfigure >>>>>>>>>> >>>>>>>>>> group = smsc >>>>>>>>>> smsc-id = A >>>>>>>>>> >>>>>>>>>> -> restart_smsc(A) >>>>>>>>>> >>>>>>>>>> first found smsc-id = A will be restarted but the second >>>>>>>>>> will not found in the config >>>>>>>>>> and error will be logged, leaving the second and third >>>>>>>>>> instances. This is not a expected behavior. >>>>>>>>>> I would expect that all 3 instances will be shutdown and >>>>>>>>>> only one will be running after restarting. >>>>>>>>>> >>>>>>>>>> I propose to change it in following sequence: >>>>>>>>>> - get lock >>>>>>>>>> - shutdown all instances with this id >>>>>>>>>> - start all found instances from new config >>>>>>>>>> - release lock >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> Alex >>>>>>>>>> >>>>>>>>>> Am 08.07.2009 um 16:30 schrieb Alejandro Guerrieri: >>>>>>>>>> >>>>>>>>>>> Commited to CVS. >>>>>>>>>>> >>>>>>>>>>> Regards, >>>>>>>>>>> -- >>>>>>>>>>> Alejandro Guerrieri >>>>>>>>>>> [email protected] >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 08/07/2009, at 15:12, Alexander Malysh wrote: >>>>>>>>>>> >>>>>>>>>>>> no objections from me... >>>>>>>>>>>> >>>>>>>>>>>> Thanks, >>>>>>>>>>>> Alex >>>>>>>>>>>> >>>>>>>>>>>> Am 08.07.2009 um 14:52 schrieb Alejandro Guerrieri: >>>>>>>>>>>> >>>>>>>>>>>>> Here's the patch with the userguide part. >>>>>>>>>>>>> >>>>>>>>>>>>> If no objections, I'll commit it later. >>>>>>>>>>>>> >>>>>>>>>>>>> Regards, >>>>>>>>>>>>> -- >>>>>>>>>>>>> Alejandro Guerrieri >>>>>>>>>>>>> [email protected] >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > >
