search

praekeltfoundation / vumi

Messaging engine for the delivery of SMS, Star Menu and chat messages to diverse audiences in emerging markets and beyond.
BSD 3-Clause "New" or "Revised" License
421 stars 131 forks source link

SMPP client should not read more data until messages are fully processed. #1042

Closed hodgestar closed 6 years ago

hodgestar commented 8 years ago

Currently the SMPP client can read an unlimited amount of data / PDUs from the server before fully processing a message, resulting in large memory usage caused by temporary storage needed for processing the PDUs and potentially large numbers of connections to external systems (e.g. Redis, Riak).

Twisted's structure where data read methods can't return deferreds make this an easy mistake to make, but http://twistedmatrix.com/documents/current/api/twisted.protocols.policies.ThrottlingFactory.html might help fix the issue.

See https://github.com/praekelt/vumi/blob/develop/vumi/transports/smpp/protocol.py#L226 for where a PDU is consumed by no deferred is tracked to make sure that the work is completed before more data is read.

smn commented 8 years ago

Another thing I was thinking of: does the SMPP protocol allow the client to send throttle PDUs to the server as push back?

komuw commented 6 years ago

@smn I do not think the SMPP protocol allows the client to send throttle PDU's to server, at least from my reading of the version 3.4 spec (I've never read version 5 spec, so)

In the spec(3.4) it says: "ESME_RTHROTTLED - Throttling error (ESME has exceeded allowed message limits)"

The fact that the spec adds in brackets, ESME has exceeded ... seems to imply that it only applies to clients talking to servers and not the other way.

smn commented 6 years ago

@komuw yeah, afaik it doesn't. It means you'll need to be able to exercise a lot of throttling control at the socket level to make this work. This is not going to be solved anymore in Vumi as it's currently not receiving any active development.