Duplicate txn.call_after calls when transactions are retried

[squahtx]

Synapse provides the ability for database transactions to defer some processing until after the transaction succeeds or fails, via the Cursor.call_after and Cursor.call_on_exception methods. These methods append the given callback to an internal list which gets run once the transaction is done.

When a transaction fails with a SerializationFailure, Synapse will retry it up to 5 times (inside new_transaction). Although a new Cursor is created for each attempt, the same after_callbacks list is reused. This means that if a transaction fails, then succeeds on the next attempt, the after_callbacks from the previous failed attempt will also run.

These callbacks are mostly used for cache invalidation, so this is probably okay?

---

Note that MultiWriterIdGenerator needs either the call_after or call_on_exception callbacks from failed transaction attempts to run, otherwise it wouldn't know when the ids handed out to failed transactions are done. If we were to clear out after_callbacks in between attempts, MultiWriterIdGenerator would break.

[clokep]

Does this need discussion of what the proper behavior should be?

[squahtx]

Probably? Let's stick it in the needs-discussion bucket.

[callahad]

Conclusion from meeting: let's note this behavior in in the call_* function docstrings, but leave the behavior unchanged.

There are legitimate uses for both modalities, so if we wanted to address it we'd likely want to extend the API surface to indicate whether duplicate callbacks should be enqueued or not. But we'll hold off on that until we have a concrete motivation to make the change.