Optional sync sequence number #581
Conversation
3021c0f to
31c6832
Compare
|
Ok so we must not fill out the sequence number until the final batch for the transaction. |
|
(Sorry I meant for that to be a question. :D ) |
Well in principle I could bring back the (table_1, origin-A, None, false) <- still don't know the sequence number for this transaction
(table_2, origin-A, Some(1000), false) <- here you find out the sequence number and can start passing it along
(table_3, origin-A, Some(1000), true) <- the transaction is now complete (last = true)Then we'd require that at least one of the messages in the transaction contains the sequence number. Let me know if this would simplify things for you somehow. |
|
No I think it seems simpler to do it as you have it. I just wanted to confirm |
| bool last = 6; | ||
| // Monotonically-increasing transaction number. | ||
| // Only specified in the last message of a transaction | ||
| optional uint64 sequence_number = 5; |
There was a problem hiding this comment.
(just noting this won't be bw compatible with existing clients)
| if origin != tx.origin { | ||
| return Err(SyncError::InvalidMessage { | ||
| reason: format!( | ||
| "Transaction from a new origin ({origin}) started without finishing the transaction from the previous one ({})", | ||
| tx.origin | ||
| ) | ||
| }); |
There was a problem hiding this comment.
This can happen during failover though? @eatonphil
There was a problem hiding this comment.
Yes I think I commented about this in tests Marko mentioned me on. You should not panic. While it's an invalid scenario it is certainly a possible scenario. You should not crash or become unrecoverable from this.
There was a problem hiding this comment.
Another example is not even failover but just if the writer process itself crashes midway through. We may not send the full transaction and then may send it again (from the same origin).
There was a problem hiding this comment.
Note that the above merely returns an error to the caller, and doesn't panic/crash the process. Then presumably the other side can re-adjust it's position in the message queue and try again?
| // sync are actually redundant (since they were already persisted) and skip them. | ||
| // Also create a new `SyncCommitInfo` that corresponds to the last full transaction in the | ||
| // pending changes, | ||
| fn skip_syncs<'a>( |
There was a problem hiding this comment.
Didn't @eatonphil mention something about how we can't actually skip over messages with an LSN < last committed because they might have changed?
There was a problem hiding this comment.
Yes I still believe you must not do that. cc @vadim4o
67fb7bb to
2156dc0
Compare
bc5304d to
7e96f81
Compare
Make sequence numbers optional in the protocol, and don't rely on their presence for the flushing mechanism to work. The reason for this is that PGD might not know the sequence number until the very end (last message) of the transaction.
This is achieved with a single queue of pending transactions, whereby with bookkeeping we:
Somesequence number)