Normalize plan to increase the plan apply throughput

[arshjohar]

This PR adds normalization of the plan to commit only the diff for stopped and preempted allocs to the raft log to enable better throughput. It also starts using omitempty on some of the structs during msgpack serialization to omit the empty fields.

[arshjohar]

Continuing the discussion from
https://github.com/hashicorp/nomad/pull/5407/files/c242adea8786e7541c8c108c342026730b028ff4#diff-ccbd515c67aa55098b48f1106de134aaR4134

There are a few reasons I don't think returning an error is the right approach here:

1. We do diffs for only stopped or preempted allocs. If they don’t exist, it means they have already been stopped/preempted. If that's the case, it’s fine for the update to not make it to the alloc in that case.
2. The raft log has already been dispatched at this point, and this code just applies the log to the state store. I'm not sure it'd even be 'correct' to not apply this to the state store, given that we can't remove the raft log entry. Wouldn't the state store be stuck whenever it has to apply this log entry?
3. It looks like existing code for preempted allocs dealt with this in the same way.

   nomad/nomad/state/state_store.go

   Line 236 in 987ed01

   if existing == nil {

[preetapan]

1. should not happen unless the plan applier is acting on stale state store data. We asked about returning an error so that its very visible when this happens. ie. this suggestion was for safety

Re: 2) - returning the error will fail the optimistic apply to the leader's state store in

nomad/nomad/plan_apply.go

Line 226 in 95297c6

if snap != nil {

and propagate up to the worker in

nomad/nomad/worker.go

Line 324 in 95297c6

if err := w.srv.RPC("Plan.Submit", &req, &resp); err != nil {

, eventually that will fail the evaluation and retry upon which the scheduler can make a new plan with updated information. The state store will always fail on that entry, but a future entry with correct information should succeed. Also see

nomad/nomad/plan_queue.go

Line 20 in 95297c6

type PlanFuture interface {

and how we block on the future from raft in

nomad/nomad/plan_endpoint.go

Line 43 in 95297c6

result, err := future.Wait()

.

Re: 3) that logic could be worth changing as well, I will address that in a future PR since I am going to be touching the plan applier again after this PR is merged, you don't need to change that right now though.

[schmichael]

Suggested change

	var MinVersionPlanNormalization = version.Must(version.NewVersion("0.9.1"))
	var MinVersionPlanNormalization = version.Must(version.NewVersion("0.9.2"))

[preetapan]

LGTM, great work overall

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.