5 ms·

> Python chooses to provide (3) because Python is slow anyway so why not at least provide the least surprising container I believe this misses quite an importa

by bostik 2y ago

> Python chooses to provide (3) because Python is slow anyway so why not at least provide the least surprising container I believe this misses quite an important bit of history. Python's dict retains key creation order since 3.6, and the behaviour was made official from 3.7 onwards. But before that, the keys were in unspecified order. Not random, because the order was stable: if you iterated through the same dict twice within the same process, you got the keys in the same order. The property of retaining key creation order was a side effect from the underlying implementation. In the 3.6 release, Python switched over to their new dictionary implementation, lifted from the PyPy project. From what I recall, the reason for the change was that the new implementation had a notably lower per-key overhead. That decrease in memory use resulted, I believe, in the slightly faster performance as well. Order retention came "for free". Personally I believe that order retention as a default property is a mistake. Now, I admit that OrderedDict semantics are often more convenient, but they break from the expected dict/map semantics with other languages. And since we are stating our opinions, in my mind Go choosing to forcibly randomise maps' key traversal order is a good safeguard. It guarantees that no-one can even accidentally depend on key iteration order. (Yes, I have seen production outages thanks to someone's code implicitly relying on key creation/traversal order when processing RESTful payloads.) As should be apparent, I disagree with the current behaviour being "least surprising".

fuzztester 2y ago
>(Yes, I have seen production outages thanks to someone's code implicitly relying on key creation/traversal order when processing RESTful payloads.) Interesting. Example of that?
- bostik 2y ago
  Sure. This is from the previous job. And for background: the service in question would routinely have several thousand concurrent, live user sessions. All stateful. Two teams, let's call them Team A and team B, would have their respective services handling user traffic. Service maintained by Team A would handle the traffic, while service maintained by Team B would handle the more complex background state transitions that Team A would not have to care about during the day. Service A would hold a complete client session state. Service B was stateless. Messages sent from service A to service B would contain all the necessary data to build up the correct state for every message received. Team A wrote their service in "not Python" language. Team B wrote theirs in Python. Communication between the services was RESTful, so essentially "JSON payload in a HTTP POST message". Service B had a construction in their code that in simplified terms looked a bit like this: data = json.loads(msg.data) for key_, val_ in data.items(): do_stuff(key_, val_) And then inside the do_stuff() routine, there was a piece of logic that used an implicit state machine. It wasn't written like one, but it happened to rely on the processing order... Like this: def do_stuff(field, vals): if field in (<possible action triggers>): # do something based on field else: # other things Because service B was written in Python, and this was post python 3.6 days, the 'data' read from the message created a dictionary with keys in the same order they happened to come off the wire. Everything worked fine, because the way the on-the-wire JSON payload at service A was constructed also happened to put field keys in a specific order. Service B could process the fields in the order they came through and could build a larger state internally based on each of the fields. Then, as happens to every well used service, requirements change. In order to support new use cases, service A would need to include a new field - and for the maintainers of that service, the most logical place in their internal structure was between existing fields. This change was known, and service B had added support for this additional field. In 'do_stuff' internals, they had added the new field to the end of the possible action triggers. They also had unit tests - written by themselves - to ensure their service would work correctly whether it received old or new payloads. The unit tests had added the new field after the existing fields in their test inputs. Their internal state machine was coherent and correct. And then, Team A ships their new service. Service B promptly starts to crash. Every crash triggers Sentry client to serialise the full stack trace and send it over. In order to prevent a cascade failure, Sentry itself has been configured with a throttle, so once enough in-flight request are lined up, it starts to apply backpressure and response delays. Sentry clients within service B end up blocking their respective workers. Service A can not reliably send its message over, because service B is bogged down waiting for N+1 Sentry client submissions to complete. In order to capture the error situations properly, service A also has Sentry client within it... It takes about 10 minutes for the teams to figure out what's going on before team A rolls back their deployment. But that was nonetheless visible downtime during live trading hours. The root cause was obviously a logic bug, but it was only possible to build up to having such a logic bug due to the key iteration order semantics.
  - Zamiel_Snawley 2y ago
    What a great example, thank you for writing it!

by bostik 2y ago

fuzztester 2y ago

>(Yes, I have seen production outages thanks to someone's code implicitly relying on key creation/traversal order when processing RESTful payloads.) Interesting. Example of that?

bostik 2y ago
Sure. This is from the previous job. And for background: the service in question would routinely have several thousand concurrent, live user sessions. All stateful. Two teams, let's call them Team A and team B, would have their respective services handling user traffic. Service maintained by Team A would handle the traffic, while service maintained by Team B would handle the more complex background state transitions that Team A would not have to care about during the day. Service A would hold a complete client session state. Service B was stateless. Messages sent from service A to service B would contain all the necessary data to build up the correct state for every message received. Team A wrote their service in "not Python" language. Team B wrote theirs in Python. Communication between the services was RESTful, so essentially "JSON payload in a HTTP POST message". Service B had a construction in their code that in simplified terms looked a bit like this: data = json.loads(msg.data) for key_, val_ in data.items(): do_stuff(key_, val_) And then inside the do_stuff() routine, there was a piece of logic that used an implicit state machine. It wasn't written like one, but it happened to rely on the processing order... Like this: def do_stuff(field, vals): if field in (<possible action triggers>): # do something based on field else: # other things Because service B was written in Python, and this was post python 3.6 days, the 'data' read from the message created a dictionary with keys in the same order they happened to come off the wire. Everything worked fine, because the way the on-the-wire JSON payload at service A was constructed also happened to put field keys in a specific order. Service B could process the fields in the order they came through and could build a larger state internally based on each of the fields. Then, as happens to every well used service, requirements change. In order to support new use cases, service A would need to include a new field - and for the maintainers of that service, the most logical place in their internal structure was between existing fields. This change was known, and service B had added support for this additional field. In 'do_stuff' internals, they had added the new field to the end of the possible action triggers. They also had unit tests - written by themselves - to ensure their service would work correctly whether it received old or new payloads. The unit tests had added the new field after the existing fields in their test inputs. Their internal state machine was coherent and correct. And then, Team A ships their new service. Service B promptly starts to crash. Every crash triggers Sentry client to serialise the full stack trace and send it over. In order to prevent a cascade failure, Sentry itself has been configured with a throttle, so once enough in-flight request are lined up, it starts to apply backpressure and response delays. Sentry clients within service B end up blocking their respective workers. Service A can not reliably send its message over, because service B is bogged down waiting for N+1 Sentry client submissions to complete. In order to capture the error situations properly, service A also has Sentry client within it... It takes about 10 minutes for the teams to figure out what's going on before team A rolls back their deployment. But that was nonetheless visible downtime during live trading hours. The root cause was obviously a logic bug, but it was only possible to build up to having such a logic bug due to the key iteration order semantics.
- Zamiel_Snawley 2y ago
  What a great example, thank you for writing it!