sui
sui copied to clipboard
[narwhal] initialize GC round on recovery
CertificateWaiter
relies on tx_consensus_round_updates
to calculate the GC round, which serves as the basis for fetching certificates from remote nodes (certificates earlier than GC round will not make into the DAG). However after a node restarts, tx_consensus_round_updates
is initialized to the default value 0 and not recovered.
In practice after a primary has accumulated many certificates locally, the above is more problematic: when fetching certificates, CertificateWaiter
sends the GC round and bitmaps for the rounds where it has certificates above the GC round. When there are millions of certificates above the GC round, the server takes a long time to scan through the bitmaps of rounds to avoid sending back duplicated certificate. The CertificateWaiter
client can time out during the period, and not make any progress.
This PR moves the sender of tx_consensus_round_updates
to the consensus component, and initializes the last committed round on recovery to avoid the issue.
Also, add more debug logs to fetch_certificate
handler, and load certificates for at most 10s, to avoid clients timing out and wasting work.
And fix the calculation of commit round sent from consensus to primary. Currently it is the round of the certificate triggering commit, but this certificate is the child of the leader, not the leader itself. So the existing logic was sending committed round + 1 to primary. Fix this by using the highest round of committed certificates instead.
Great catch and thanks @mwtian for this!! 💯
For post mortem reasons, I am citing here the task that I did create back then for the gc round recovery and we never picked up #5326 . I believe it was an oversight and we should have done it (still can't remember why it hasn't been picked up), although I guess with the old certificate_waiter it was less important due to its recursive nature and checking against the store, so maybe it wasn't treated as must done.
Yes, thanks for the info. The first implementation of CertificateWaiter did not rely on gc round either.