search

p4lang / p4runtime

Specification documents for the P4Runtime control-plane API
Apache License 2.0
142 stars 88 forks source link

Clarify wording around "no-op" master arbitration updates #361

Closed smolkaj closed 2 years ago

smolkaj commented 3 years ago

Section 5.3.2.(e).ii of the spec, see attachment, states:

If the controller is a backup, this is a no-op and the role.config is ignored. No response is sent to any controller.

This seems to suggest that in this case, we shouldn't even send a reponse to the controller originating the MasterArbitrationUpdate.

Is this phrasing intended? If so, what is the motivation?

Sending an ALREADY_EXISTS response to the controller originating the MasterArbitrationUpdate would be more consistent with the rest of the spec (in particular, section 5.4), eliminating this special case.

spec

smolkaj commented 3 years ago

@antoninbas @jonathan-dilorenzo @rhalstea for visibility

jonathan-dilorenzo commented 3 years ago

If I am correctly understanding your suggestion, you want to remove all of 5.3.2.(e) since the information touched on is covered just below, except that this induces a spec change where 5.3.2.(e).ii is no longer a no-op.

This makes sense to me.

smolkaj commented 3 years ago

Thanks for clarifying. Indeed this is the intention. Though I think we still need to keep 5.3.2.(e) to say how to update role.config.

jonathan-dilorenzo commented 3 years ago

I think we shouldn't need that because it seems to be covered by the first point of "If the MasterArbitrationUpdate is accepted":

"If election_id is greater than or equal to election_id_past, then the controller becomes primary. The server updates the role configuration to role.config for the given role.id."

smolkaj commented 3 years ago

Good point. We may still have to say when the MasterArbitrationUpdate is accpeted in 5.3.2.(e) then, I believe. Maybe we can merge (e) and (f).