Hi Werner,
Do you mean something like this (ignore white-space differences)?
Index: gw/smsbox.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsbox.c,v
retrieving revision 1.284
diff -u -b -r1.284 smsbox.c
--- gw/smsbox.c 6 Dec 2009 17:24:14 -0000 1.284
+++ gw/smsbox.c 1 Mar 2010 01:53:25 -0000
@@ -1075,7 +1075,7 @@
unsigned long retries;
int method;
- while ((id = gwlist_consume(smsbox_http_requests)) != NULL) {
+ while (wlist_producer_count(smsbox_http_requests) > 0) {
/*
* Sleep for a while in order not to block other operting requests.
* Defaults to 10 sec. if not given via http-queue-delay directive in
@@ -1084,6 +1084,7 @@
if (http_queue_delay > 0)
gwthread_sleep(http_queue_delay);
+ while ((id = gwlist_consume(smsbox_http_requests)) != NULL) {
debug("sms.http",0,"HTTP: Queue contains %ld outstanding requests",
gwlist_len(smsbox_http_requests));
@@ -1109,6 +1110,7 @@
http_destroy_headers(req_headers);
octstr_destroy(req_body);
}
+ }
}
Would re-queued events be retried without the sleep? That is, if we had
1 queued connection failure would we end up retrying 3 times one after
the other (without the 10second wait). So we would want to sleep after
each pass of the (original) list?
Cheers,
Alan
p.s full patch with indenting attached.
Werner Coetzee wrote:
> Hi Alejandro
>
>
> Yes it is a bug.
> We have the same problem, and if you look at the code you'll clearly see the
> problem.
>
> Look at the http_queue_thread function in smsbox.c.
>
> It consumes 1 msg from smsbox_http_requests, then sleeps http_queue_delay,
> then consumes 1 message, then sleeps http_queue_delay and on and on it goes,
> 1 message at a time.
> So if there are 100 messages that needs to be retried, and your delay is 1
> minute, it will take 100 minutes before it even gets to the last msg in the
> retry queue, very bad IMO.
>
> It will be easy to fix to just put a while loop around the current while loop
> and move the sleep outside the consume while loop, e.g.:
>
>
> while (gwlist_producer_count(smsbox_http_requests) > 0) {
> if (http_queue_delay > 0)
> gwthread_sleep(http_queue_delay);
>
> while ((id = gwlist_consume(smsbox_http_requests)) != NULL) {
> // do the retry stuff
> }
> }
>
>
> Unfortunately I'm not on cvs kannel, so I can't provide a patch.
>
>
> Regards
> Werner
>
>
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of
> Alejandro Guerrieri
> Sent: 25 February 2010 19:26
> To: Arkadiy Kulev
> Cc: kannel_dev_mailinglist Devel
> Subject: Re: Re[2]: reenable HTTP_MAX_RETRIES and HTTP_RETRY_DELAY for dlr-url
>
> 1. I hope so! At this stage we're just researching if is this actually a bug
> or maybe some misconfiguration on our side. If it proves to be a bug I'll try
> to fix it of course.
>
> 2. Afaik, smsbox doesn't actually use the store, but yes I agree that some
> persistence mechanism is needed.
>
> Regards,
> --
> Alejandro Guerrieri
> [email protected]
>
>
>
> On 25/02/2010, at 18:08, Arkadiy Kulev wrote:
>
>
>> Hello Alejandro,
>>
>> 1. Can this be fixed? (only one in 30 seconds is not good).
>> 2. Can you put DLRs in store just in case smsbox dies?
>>
>> I suppose reliability in problem cases should be a issue here.
>>
>>
>>
>>
>>
>>
>> Thursday, February 25, 2010, 7:42:20 PM, you wrote:
>>
>>
>>> Alex,
>>>
>>> We have those parameters configured, and what Juan describes is what's
>>> actually happening:
>>>
>>> If you configure it to retry X times every 30 seconds, and the app
>>> handling the DLR's doesn't answer, the DLR's are kept _in memory_
>>> (not the store, so if you shutdown smsbox you lose them) and when
>>> the app starts answering they are retried, but one every 30 seconds.
>>>
>>> Regards,
>>> --
>>> Alejandro Guerrieri
>>> [email protected]
>>>
>>
>>
>>> On 25/02/2010, at 17:32, Alexander Malysh wrote:
>>>
>>>> Hi Juan,
>>>>
>>>> as far as I see in source code, DLRs handled the same. So please try with
>>>> those 2 config options.
>>>>
>>>> Thanks,
>>>> Alexander Malysh
>>>>
>>>> Am 25.02.2010 um 14:54 schrieb Juan Nin:
>>>>
>>>>
>>>>> Alex, I'm not sure if you were that night on Barcelona with Alejandro,
>>>>> Stipe, etc, but I asked him to ask you guys about DLR retries...
>>>>> DLR retries don't look at all into http-request-retry and
>>>>> http-queue-delay, or at least don't follow the expected behavior.
>>>>>
>>>>> Instead, each DLR that failed to be posted to it's dlr-url gets
>>>>> retried in a fashion of 1 DLR every 30 seconds. Not each separate DLR
>>>>> every 30 seconds, but _only_ 1 DLR every 30 seconds, which is
>>>>> obviously not good at all, since if you got many queued DLRs that
>>>>> failed to be posted, then they will take an eternity to be retried...
>>>>>
>>>>> And what was replied to Alejandro was that no doubt it was a bug.
>>>>>
>>>>> do you think this is something that could easily be fixed?
>>>>>
>>>>>
>>>>> On Thu, Feb 25, 2010 at 7:45 AM, Alexander Malysh <[email protected]>
>>>>> wrote:
>>>>>
>>>>>> Hi Arkadiy,
>>>>>> why don't you read userguide?
>>>>>> http-request-retry integer If set, specifies how many retries should be
>>>>>> performed for failing HTTP requests of sms-services. Defaults to 0, which
>>>>>> means no retries should be performed and hence no HTTP request queuing is
>>>>>> done.
>>>>>> http-queue-delay integer If set, specifies how many seconds should pass
>>>>>> within the HTTP queuing thread for retrying a failed HTTP request.
>>>>>> Defaults
>>>>>> to 10 sec. and is only obeyed if http-request-retry is set to a non-zero
>>>>>> value.
>>>>>> Thanks,
>>>>>> Alexander Malysh
>>>>>> Am 25.02.2010 um 10:26 schrieb Arkadiy Kulev:
>>>>>>
>>>>>> Hello Guys,
>>>>>>
>>>>>> I am wondering, why are HTTP_MAX_RETRIES and HTTP_RETRY_DELAY
>>>>>> disabled in gw/smsbox.c?
>>>>>>
>>>>>> For instance, I use dlr-url to keep track of my deliveries (the url
>>>>>> itself has an internal ID of the message that I use in my software).
>>>>>>
>>>>>> What if my HTTP server goes down? I loose the notification.
>>>>>>
>>>>>> The access.log doesn't show the dlr-url, so I can't understand which
>>>>>> of the messages correspond to which message ids in my software.
>>>>>>
>>>>>> It would be cool, if I could configure kannel to keep resending the
>>>>>> DLR to my HTTP server every N seconds for Y times (or maybe forever),
>>>>>> until it gets through.
>>>>>>
>>>>>> So why is it disabled?
>>>>>>
>>>>>> Arkadiy Kulev mailto:[email protected]
>>>>>> +7 495 5070602
>>>>>> Moscow, Russia
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> Juan Nin
>>>>> 3Cinteractive / Mobilizing Great Brands
>>>>> http://www.3cinteractive.com
>>>>>
>>>>>
>>>>
>>
>>
>> Arkadiy Kulev mailto:[email protected]
>> +7 495 5070602
>> Moscow, Russia
>>
>>
>
>
> --
>
> PLEASE CONSIDER THE ENVIRONMENT BEFORE PRINTING THIS E-MAIL.
>
> This e-mail message and all attachments transmitted with it are confidential
> and are intended solely for the addressee(s). If the reader of this message
> is not the intended recipient, you are hereby notified that any reading,
> dissemination, distribution, copying, or other use of this message or its
> attachment(s) is strictly prohibited.
>
>
>
Index: gw/smsbox.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsbox.c,v
retrieving revision 1.284
diff -u -B -r1.284 smsbox.c
--- gw/smsbox.c 6 Dec 2009 17:24:14 -0000 1.284
+++ gw/smsbox.c 1 Mar 2010 01:54:31 -0000
@@ -1075,7 +1075,7 @@
unsigned long retries;
int method;
- while ((id = gwlist_consume(smsbox_http_requests)) != NULL) {
+ while (wlist_producer_count(smsbox_http_requests) > 0) {
/*
* Sleep for a while in order not to block other operting requests.
* Defaults to 10 sec. if not given via http-queue-delay directive in
@@ -1084,30 +1084,32 @@
if (http_queue_delay > 0)
gwthread_sleep(http_queue_delay);
- debug("sms.http",0,"HTTP: Queue contains %ld outstanding requests",
- gwlist_len(smsbox_http_requests));
+ while ((id = gwlist_consume(smsbox_http_requests)) != NULL) {
+ debug("sms.http",0,"HTTP: Queue contains %ld outstanding requests",
+ gwlist_len(smsbox_http_requests));
- /*
- * Get all required HTTP request data from the queue and reconstruct
- * the id pointer for later lookup in url_result_thread.
- */
- get_receiver(id, &msg, &trans, &method, &req_url, &req_headers, &req_body, &retries);
+ /*
+ * Get all required HTTP request data from the queue and reconstruct
+ * the id pointer for later lookup in url_result_thread.
+ */
+ get_receiver(id, &msg, &trans, &method, &req_url, &req_headers, &req_body, &retries);
+
+ if (retries < max_http_retries) {
+ id = remember_receiver(msg, trans, method, req_url, req_headers, req_body, ++retries);
- if (retries < max_http_retries) {
- id = remember_receiver(msg, trans, method, req_url, req_headers, req_body, ++retries);
+ debug("sms.http",0,"HTTP: Retrying request <%s> (%ld/%ld)",
+ octstr_get_cstr(req_url), retries, max_http_retries);
- debug("sms.http",0,"HTTP: Retrying request <%s> (%ld/%ld)",
- octstr_get_cstr(req_url), retries, max_http_retries);
+ /* re-queue this request to the HTTPCaller list */
+ http_start_request(caller, method, req_url, req_headers, req_body,
+ 1, id, NULL);
+ }
- /* re-queue this request to the HTTPCaller list */
- http_start_request(caller, method, req_url, req_headers, req_body,
- 1, id, NULL);
+ msg_destroy(msg);
+ octstr_destroy(req_url);
+ http_destroy_headers(req_headers);
+ octstr_destroy(req_body);
}
-
- msg_destroy(msg);
- octstr_destroy(req_url);
- http_destroy_headers(req_headers);
- octstr_destroy(req_body);
}
}
