Looks sound. Some advice not necessarily applying to this quite simple setup, but from experience:
1- always (always) change the "HA-group-id" to something other than the default "0"! this will determine the virtual MAC addresses used for the interfaces. This parameter is CLI-only.
2- equal priorities and no "override enable" setting - this way, when a failover occurs, there will be no fallback to the original primary, thus avoiding a second interruption
3- "set uninterruptable enable" which might already be enabled per default
4- "session-pickup": yes, for TCP sessions only. UDP sessions are way less critical and do not cause a huge overhead when they have to be restarted, so I prefer "connectionless disable". IPsec sessions always break on failover.
Session sync increases both the traffic volume on the HA links and CPU load. This setting should not be set "per default" but deliberately.
5- no encryption on the HA link(s). Unless the cluster units are located far apart via WAN lines. Increases CPU load.
6- by default, HA monitoring will detect link failure, in addition to device failure. In a switched environment, links can stay up forever even though the line is broken further up. Install ping target monitoring (system link-monitor) to ensure a WAN line really is up. Either choose the ISP's gateway (preferably it's loopback IP) or number the WAN line and ping the other end (for instance, with VPNs).
"Kernel panic: Aiee, killing interrupt handler!"