Re: [PATCH]: HTTP reply from sendota fixes (#1)

Hi,

patch looks good. one thing: please fix coding style (indents).
Otherwise I'm +1 for it.

Thanks,
Alex

Dziugas Baltrunas wrote:

> Hi again,
> 
> any objections on this? I've already tested the patch on bigger loads
> and it seems to work fine.
> 
> Sometimes you can see "Delayed reply - wait for bearerbox" after " Got
> ACK" in the logs, but this is obvious since we print the message
> before puting client into dict.
> 
> On 3/21/06, Dziugas Baltrunas <[email protected]> wrote:
>> Hi,
>>
>> attached is an attempt to follow Alexander's advice long time ago for
>> Kalle to put client into dict before sending a message to bearerbox.
>> Not sure if it will work in all cases, so additional testing is
>> required. Patch also includes fix for freeing msg which I sent before.
>>
>> To be more precise, I'm not sure if dict_remove(client_dict,
>> stored_uuid) in my patch is necessary since smsbox.c:send_message()
>> _never_ returns -1, although it's stated so and there are such checks
>> in the code. This becomes more obvious when we look to
>> write_to_bearerbox(), which returns void, so we actually never really
>> know, whether bearerbox accepted the message or not.
>>
>> Thanks.
>>
>> On 3/21/06, Dziugas Baltrunas <[email protected]> wrote:
>> > Hi, list,
>> >
>> > attached patch fixes freeing of msg structure in smsbox.c sendota
>> > stuff before extracting it's uuid with store_uuid(). Without patch
>> > applied you can see the following in the logs:
>> >
>> > 2006-03-20 18:35:36 [29721] [3] DEBUG: Stored UUID
>> > 00000000-0000-0000-0000-000000000000
>> >
>> > However, current patch does not solve HTTP reply from sendota
>> > completely, since it's related how fast we get an ACK from bearerbox.
>> > If we're successful, we'll get "0: Accepted for delivery", and if not,
>> > no answer will be sent at all. This can be illustrated by these
>> > snippets:
>> >
>> > 1. We got an ACK from bearerbox (read_messages_from_bearerbox())
>> > before store_uuid() was called:
>> >
>> > 2006-03-21 12:47:37 [19015] [3] INFO: /cgi-bin/sendota <<default>>
>> > <XXX> 2006-03-21 12:47:37 [19015] [3] DEBUG: message length 446,
>> > sending 4 messages 2006-03-21 12:47:37 [19015] [0] DEBUG: Got ACK (0)
>> > of 8b37ca04-896f-4510-8d2d-000289b6b8b2
>> > 2006-03-21 12:47:37 [19015] [0] DEBUG: No client - multi-send or ACK
>> > to pull-reply
>> > 2006-03-21 12:47:37 [19015] [3] DEBUG: Stored UUID
>> > 8b37ca04-896f-4510-8d2d-000289b6b8b2
>> > 2006-03-21 12:47:37 [19015] [3] DEBUG: Status: 202 Answer: <Sent.>
>> > 2006-03-21 12:47:37 [19015] [3] DEBUG: Delayed reply - wait for
>> > bearerbox
>> >
>> > 2. We got an ACK from bearerbox after store_uuid() was called (as
>> > expected):
>> >
>> > 2006-03-21 12:47:09 [19015] [3] INFO: /cgi-bin/sendota <<default>>
>> > <XXX> 2006-03-21 12:47:09 [19015] [3] DEBUG: message length 446,
>> > sending 4 messages 2006-03-21 12:47:09 [19015] [3] DEBUG: Stored UUID
>> > f6b0cb63-b561-49fa-951b-09aa5177067e
>> > 2006-03-21 12:47:09 [19015] [3] DEBUG: Status: 202 Answer: <Sent.>
>> > 2006-03-21 12:47:09 [19015] [3] DEBUG: Delayed reply - wait for
>> > bearerbox 2006-03-21 12:47:09 [19015] [0] DEBUG: Got ACK (0) of
>> > f6b0cb63-b561-49fa-951b-09aa5177067e
>> >
>> > This is relevant only when immediate_sendsms_reply is false (which is
>> > default!), so I should ask Kalle,  the author of introducing this
>> > feature, comment on possible thread synchronisation issues, which IMHO
>> > sometimes could also arise for ordinary sendsms request.
>> >
>> > Thanks,
>> > Dziugas
>> >
>> >
>> >
>>
>>
>> --
>> Dziugas
>>
>>
>>
> 
> 
> --
> Dziugas

-- 
Thanks,
Alex