Record Crucible region ports in CRDB #5955
Conversation
It was a mistake not to record the port that an Agent assigns a Region when it's created, and this PR rectifies that: ports are recorded when regions are ensured, and a background task will scan for regions that are missing a port, and attempt to contact the appropriate Agent in order to fill that in. Fixes oxidecomputer#5846
| ) | ||
| let maybe_addr = osagactx | ||
| .nexus() | ||
| .region_addr(log, params.request.old_region_id) |
There was a problem hiding this comment.
This is calling the function defined in nexus/src/app/crucible.rs, which tries to access the port and set if if we don't have the value, right?
If this is already doing the work of contacting the agent and back-filling the port, why are we bothering to handle the match maybe_addr => None case below? Isn't that redundant with the work in nexus/src/app/crucible.rs's region_addr function?
There was a problem hiding this comment.
In order for region replacement to continue even if the agent is gone, this saga node makes the assumption that if there's a unique target with the dataset's address in the region set, then that's the region's port. It's not redundant in that there's an assumption made, not a lookup.
However, if it's good enough to make this assumption for region replacement, the question then becomes: if there is only one target using the dataset's address, should that target's port be recorded in the database as this region's port? I'm thinking yes.
There was a problem hiding this comment.
Ack, the structure makes more sense to me. It's basically:
- Backfill the port info if we can
- If we can't, at least confirm that the address is known to be unique
- If we can't do that, bail
| ) | ||
| let maybe_addr = osagactx | ||
| .nexus() | ||
| .region_addr(log, params.request.old_region_id) |
There was a problem hiding this comment.
Ack, the structure makes more sense to me. It's basically:
- Backfill the port info if we can
- If we can't, at least confirm that the address is known to be unique
- If we can't do that, bail
Co-authored-by: Sean Klein <[email protected]>
It was a mistake not to record the port that an Agent assigns a Region when it's created, and this PR rectifies that: ports are recorded when regions are ensured, and a background task will scan for regions that are missing a port, and attempt to contact the appropriate Agent in order to fill that in.
Fixes #5846