Menu
▾
▴
#32 Message handling doesn't use transactions
I haven't actually worked through the implications of this - it might be harmless. However, server message handling is done in stages:
1. Process the message.
2. Record in the database that the message was processed.
If the server crashes between these steps, the fact that the message was received could be lost. These steps need to be executed as a single database transaction.
s/message was received/message was processed/. So the message might get reprocessed, and who knows what will happen.
I'm starting to think that messages from clients get processed in far, far too many phases with separate DbCons. Ideally, I would rewrite a big chunk of it like this (specifically for messages that modify something, as opposed to queries).
1. Message received from a client or a server (use the same format for both).
2. If the message comes from a client, add a timestamp and a username to it, otherwise use an incoming timestamp.
3. If the message comes from a server, open a DbCon, record the message and mark it unprocessed, and acknowledge the message (this allows ACKs to be sent early, although it might not be worth it).
4. Open a DbCon and start a transaction.
5. If the message came from a client, do safety checks on it. If it came from another server, assume that server has accepted it and it's really too late to try to do safety checks.
6. Apply the changes requested by the message (in the transaction).
7. Record the message as processed.
8. Commit the transaction.
9. If the message came from a client, reply with ok if the transaction was committed successfully, or err if it was rolled back for some reason (permission check failed, database error). A try/catch might work nicely for the error handling.
Currently all server messages are processed serially anyway (in the message handler thread), so using transactions shouldn't have any negative impact on performance. It may even improve since there would potentially be no further need for a separate message handler thread: each message would be handled entirely in a worker thread. And avoiding all the code that takes a MessageBlock and rewrites it into a Message would simplify the code a lot, although I haven't looked at whether they're always carrying the same information.
Rolling back transactions might cause holes in the allocation of unique IDs (because they're allocated by the C++ code, not an AUTOINCREMENT column), but I don't think that should have any ill effects.