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

broker: parent-uri attribute is set for instance with no parent #5622

Closed
grondo opened this issue Dec 13, 2023 · 7 comments
Closed

broker: parent-uri attribute is set for instance with no parent #5622

grondo opened this issue Dec 13, 2023 · 7 comments

Comments

@grondo
Copy link
Contributor

grondo commented Dec 13, 2023

I'm not certain, but this seems wrong:

$ flux start flux start flux getattr parent-uri
local:///var/tmp/grondo/flux-j6S7sS/local-0

It appears that the broker sets parent-uri whenever FLUX_URI is set in the environment, even if the enclosing instance isn't technically the parent of the new instance.

Is this expected behavior?

@garlick
Copy link
Member

garlick commented Dec 13, 2023

I think that is the expected behavior but maybe because I didn't think it through?

I guess a test instance should always be the "top" flux even if it was started by Flux? What is the fallout from the current behavior?

@grondo
Copy link
Contributor Author

grondo commented Dec 13, 2023

In this case flux did not start the test instance, it just happened to be started under an existing instance. It seems like an instance should only be considered the parent if it allocated resources to the child instance?

The fallout is that existence of the parent-uri attribute alone doesn't indicate that the current instance was launched by Flux. I guess both parent-uri and jobid need to be considered?

@grondo
Copy link
Contributor Author

grondo commented Dec 13, 2023

I also admit I haven't thought this through either, though. It is simple enough to check for a jobid attribute, and this issue was only opened to get clarification.

@garlick
Copy link
Member

garlick commented Dec 13, 2023

There is also this, which works as expected.

$ flux start flux start flux getattr instance-level
0

Yeah seems like when flux is not run as a flux job, all three of these should be true

  • instance-level is 0
  • jobid is not set
  • parent-uri is not set

@grondo
Copy link
Contributor Author

grondo commented Dec 13, 2023

I just went through the code and currently all places where parent-uri is potentially used, the jobid attribute is also checked and both must be set before an attempt to contact the parent is made. So, currently there is no fallout.

There is this comment in src/modules/resource/inventory.c

* Case 2 (method=job-info)
* ------------------------
* On the rank 0 broker, if the 'parent-uri' broker attribute is defined,
* a connection is made to the parent broker, and R is read from the
* job-info module. This R was assigned to the instance by the enclosing
* instance scheduler, and includes ranks representing brokers in the
* enclosing instance.

which probably needs a very slight tweak.

@garlick
Copy link
Member

garlick commented Jan 8, 2024

Does this seem right then?

diff --git a/src/broker/broker.c b/src/broker/broker.c
index 7bdb38bdd..c040f1cf9 100644
--- a/src/broker/broker.c
+++ b/src/broker/broker.c
@@ -629,7 +629,14 @@ static void init_attrs (attr_t *attrs, pid_t pid, struct flux_msg_cred *cred)
 {
     const char *val;
 
-    val = getenv ("FLUX_URI");
+    /* Set the parent-uri attribute IFF this instance was run as a job
+     * in the enclosing instance.  "parent" in this context reflects
+     * a hierarchy of resource allocation.
+     */
+    if (getenv ("FLUX_JOB_ID"))
+        val = getenv ("FLUX_URI");
+    else
+        val = NULL;
     if (attr_add (attrs, "parent-uri", val, ATTR_IMMUTABLE) < 0)
         log_err_exit ("setattr parent-uri");
     unsetenv ("FLUX_URI");

garlick added a commit to garlick/flux-core that referenced this issue Jan 8, 2024
Problem: the parent-uri attribute is set whenever FLUX_URI is
set in the broker environment, but this does not necessarily
indicate that the instance has a "parent" in the resource hierarchy
sense.

Suppress setting parent-uri when the broker is not running as
a Flux job, determined by checking for FLUX_JOB_ID.

Fixes flux-framework#5622
@grondo
Copy link
Contributor Author

grondo commented Jan 8, 2024

That looks correct to me.

garlick added a commit to garlick/flux-core that referenced this issue Jan 8, 2024
Problem: the parent-uri attribute is set whenever FLUX_URI is
set in the broker environment, but this does not necessarily
indicate that the instance has a "parent" in the resource hierarchy
sense.

Suppress setting parent-uri when the broker is not running as
a Flux job, determined by checking for FLUX_JOB_ID.

Fixes flux-framework#5622
garlick added a commit to garlick/flux-core that referenced this issue Jan 9, 2024
Problem: the parent-uri attribute is set whenever FLUX_URI is
set in the broker environment, but this does not necessarily
indicate that the instance has a "parent" in the resource hierarchy
sense.

Suppress setting parent-uri when the broker is not running as
a Flux job, determined by checking for FLUX_JOB_ID.

Fixes flux-framework#5622
garlick added a commit to garlick/flux-core that referenced this issue Jan 9, 2024
Problem: the parent-uri attribute is set whenever FLUX_URI is
set in the broker environment, but this does not necessarily
indicate that the instance has a "parent" in the resource hierarchy
sense.

Suppress setting parent-uri when the broker is not running as
a Flux job, determined by checking for FLUX_JOB_ID.

Fixes flux-framework#5622
@mergify mergify bot closed this as completed in f70492a Jan 9, 2024
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

No branches or pull requests

2 participants