Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DWS/shell: potentially exchange use of eventlog for static rabbitmapping file #244

Closed
jameshcorbett opened this issue Dec 10, 2024 · 2 comments · Fixed by #253
Closed

DWS/shell: potentially exchange use of eventlog for static rabbitmapping file #244

jameshcorbett opened this issue Dec 10, 2024 · 2 comments · Fixed by #253

Comments

@jameshcorbett
Copy link
Member

jameshcorbett commented Dec 10, 2024

Soon, all LC rabbit clusters will have a JSON file, /etc/flux/system/rabbitmapping, that maps compute nodes to rabbits and vice versa, structured like the following:

{
  "computes": {
    "tioga10": "tioga101",
    "tioga11": "tioga101",
    "tioga12": "tioga101",
    "tioga13": "tioga101",
    "tioga14": "tioga101",
    "tioga15": "tioga101",
    "tioga16": "tioga101",
    "tioga17": "tioga101",
    "tioga18": "tioga101",
    "tioga19": "tioga101",
    "tioga20": "tioga101",
    "tioga21": "tioga101",
    "tioga22": "tioga101",
    "tioga23": "tioga101",
    "tioga24": "tioga101",
    "tioga25": "tioga101",
    "tioga26": "tioga102",
    "tioga27": "tioga102",
    "tioga28": "tioga102",
    "tioga29": "tioga102",
    "tioga30": "tioga102",
    "tioga31": "tioga102",
    "tioga32": "tioga102",
    "tioga33": "tioga102",
    "tioga34": "tioga102",
    "tioga35": "tioga102",
    "tioga36": "tioga102",
    "tioga37": "tioga102",
    "tioga38": "tioga102",
    "tioga39": "tioga102",
    "tioga40": "tioga102",
    "tioga41": "tioga102"
  },
  "rabbits": {
    "tioga101": {
      "capacity": 30659987079168,
      "hostlist": "tioga[10-25]"
    },
    "tioga102": {
      "capacity": 30659987079168,
      "hostlist": "tioga[26-41]"
    }
  }
}

It duplicates some information set in the eventlog for the dws-environment shell plugin, which reads events like

1733860554.742788 dws_environment variables={"DW_JOB_project1":"/mnt/nnf/0f3c9edf-9c4d-4e71-a7f0-c619a97d2138-0","DW_WORKFLOW_NAME":"fluxjob-167335958938502144","DW_WORKFLOW_NAMESPACE":"default"} rabbits={"elcap310":"elcap[2745-2760]"} copy_offload=false

(note that the eventlog entry provides a rabbit: hostlist mapping for every rabbit associated with the job)

to figure out what the local rabbit is and set the FLUX_LOCAL_RABBIT environment variable. But if users can get the same information by running jq .computes.$HOSTNAME /etc/flux/system/rabbitmapping, I wonder if $FLUX_LOCAL_RABBIT is necessary. Or, alternatively, perhaps the shell plugin could read from the file to set the variable, and the eventlog could drop that (potentially very large) entry.

@jameshcorbett
Copy link
Member Author

@grondo do you have any thoughts on this / whether it sounds like a good idea?

@grondo
Copy link
Contributor

grondo commented Dec 10, 2024

Sounds like a pretty good idea to me. I'm guessing the mapping from compute node to rabbits will never change, so putting the redundant and duplicate information in the job eventlog may be wasting KVS space.

I like the idea of setting the environment variable in the shell plugin using this new config file. Then users won't see a difference when you update to the new method.

I can't think of a good reason to keep the mapping in the eventlog either. Unless that is the only way a job can see the rabbits to which it has access?

jameshcorbett added a commit to jameshcorbett/flux-coral2 that referenced this issue Jan 24, 2025
Problem: the dws_environment shell plugin no longer looks for a
mapping from rabbits to compute nodes in the 'dws_environment'
eventlog entry, so that part of the event's context (which could
be quite large) can be dropped.

Drop it.

Fixes flux-framework#244.
jameshcorbett added a commit to jameshcorbett/flux-coral2 that referenced this issue Jan 24, 2025
Problem: the dws_environment shell plugin no longer looks for a
mapping from rabbits to compute nodes in the 'dws_environment'
eventlog entry, so that part of the event's context (which could
be quite large) can be dropped.

Drop it.

Fixes flux-framework#244.
@mergify mergify bot closed this as completed in #253 Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants