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

kvs: improve API for namespaces #1435

Closed
garlick opened this issue Apr 6, 2018 · 4 comments
Closed

kvs: improve API for namespaces #1435

garlick opened this issue Apr 6, 2018 · 4 comments
Assignees

Comments

@garlick
Copy link
Member

garlick commented Apr 6, 2018

Now that we have the ns:NAMESPACE/ prefix for keys as well as flux_kvs_set_namespace(), do we need to support the FLUX_KVS_NAMESPACE environment variable?

(This was briefly noted in #1432)

After #1429 wreck sets FLUX_JOB_KVSPATH for sub-jobs. It seems like a utility, like the job shell, meant to operate on the per-job namespace. would read this and call flux_kvs_set_namespace() during its initialization? In the new execution system I was envisioning that FLUX_KVS_NAMESPACE would become something like ns:job-42/. or similar.

Just asking the question whether we need to continue supporting a special environment variable for the namespace.

In fact, maybe we should replace flux_kvs_set_namespace() wtih flux_kvs_chdir() or similar, to set a default key prefix that could optionally include both a namespace and a key part?

@chu11
Copy link
Member

chu11 commented Apr 13, 2018

Generally speaking, I like the idea of removing FLUX_KVS_NAMESPACE if its truly not needed. I think it can get tricky with the prioritization of namespace specifications (i.e. flux_kvs_set_namespace() overrides environment variable, prefix overrides flux_kvs_set_namespace()). So removing one of those would make things better.

@garlick
Copy link
Member Author

garlick commented Apr 15, 2018

Let's see how #1460 shakes out and decide which needs to go, the environment variable or flux_kvs_set_namespace().

The advantage of an environment variable is we can set it for a job and then the user can run flux kvs ... in a script to manipulate the per-job KVS space.

I'm not sure I like the flux_kvs_set_namespace() call in real world use. In the job module, where we are dealing with multiple user namespaces, it gets confusing when chaining KVS calls with continuations. They key prefix seems more intuitive in that situation.

@garlick garlick changed the title kvs: do we need FLUX_KVS_NAMESPACE? kvs: improve API for namespaces Apr 23, 2018
@chu11
Copy link
Member

chu11 commented Jan 5, 2019

Was thinking about this issue given prototyping in #1860.

flux_kvs_set_namespace() works by setting an aux value in the flux_t handle. flux_kvs_get_namespace() works by checking for the aux value OR checking for the environment variable. The aux value for a namespace takes precedence over the environment variable. If neither are set, we use "primary".

If we were a user implementing flux_kvs_copy(), it'd be really ugly setting an alternate namespace for the src and/or dst locations. We'd have to determine if an aux is set or if the environment variable is set, save the old value of the namespace set in the handle, and set it back at the end. This is ugly by itself, but add in the fact there is a chained continuation, and we have multiple error paths, and there is a lookup / commit that may have different namespaces, and it starts to be ugly.

@chu11
Copy link
Member

chu11 commented Feb 2, 2019

closing this as solved via #1977

@chu11 chu11 closed this as completed Feb 2, 2019
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