Comments (15)
This is not a fully formed idea but it seems useful for kvs_get()
and kvs_watch()
functions to adopt an interface similar to flux_rpc()
. This would enable:
- returning a
const char *json_str
where storage of the returned value is held in the response message, which in turn is held in theflux_rpc_t
like object - these functions to be used asynchronously with check/get/then interfaces, straightening out the confusing and clunky
kvs_watch()
andkvs_watch_once()
calls.
Could we just use a flux_rpc_t
directly? E.g.
flux_rpc_t *kvs_get (flux_t h, const char *key);
// can use flux_rpc_check() directly
int kvs_rpc_get (flux_rpc_t *rpc, const char **json_str);
// add get variants for different types?
// can use flux_rpc_then() directly
// can use flux_rpc_destroy() directly
from flux-core.
Watch could just be a variant of kvs_get()
with the rest identical (could even be a flag to kvs_get()
.
The RPC calls would need to be modified to support multiple responses (and multiple gets) as described in #271
from flux-core.
In order for kvs_put()
and kvs_commit()
to be used asynchronously, they could be similarly wrapped. In that case the kvs_rpc_get()
call would just return the error status (or commit may be modified to return a snapshot reference).
flux_rpc_t *kvs_put (flux_t h, const char *key, const char *json_str);
flux_rpc_t *kvs_commit (flux_t h);
from flux-core.
Hm, the get
in flux_rpc_get
is unfortunate name choice when it comes to kvs
operations, the naming here is turning out a bit confusing imo. It will be a bit difficult to read code with kvs_get
and kvs_rpc_get
s sprinkled around.
It does seem to me like you are on to something here though. It would be really nice to be able to extend the generic rpc
pattern to other services that for whatever reason need to wrap the rpc interface. That would make adding C functions wrapping the protocols much easier, as well as ensuring a consistent interface.
Oh and your proposal to have simple rpcs kvs_put
kvs_commit
return flux_rpc_t
seems exactly right to me. In general all simple rpc wrappers should follow this pattern I would think.
from flux-core.
Yeah sorry, the naming above is definitely poor. For the moment I'm kind of stumped on finding good names though - it seems like if we are wrapping the flux_rpc()
and flux_rpc_get()
calls and allowing the other flux_rpc_t
calls to be used directly, then we need to make clear in the naming both 1) the operation (kvs_get) and 2) the RPC call being wrapped (flux_rpc_get). I suppose a consolation is that this is probably about the worst case we'll have if we go this way for other flux-core RPC's. Example, flux_info()
becomes flux_info()
and flux_info_get()
- no problem!
from flux-core.
Is this a slight improvement or still "meh..."?
flux_rpc_t *kvs_request (flux_t h, const char *key);
int kvs_get_json (flux_rpc_t *rpc, const char **json_str);
int kvs_get_int (flux_rpc_t *rpc, int *val);
etc
A synchronous get would look like:
flux_rpc_t *rpc;
const char *json_str;
if (!(rpc = kvs_request (h, "foo.bar.baz"))
err_exit ("kvs_request");
if (kvs_get_json (rpc, &json_str) < 0)
err_exit ("kvs_get_json");
flux_rpc_destroy (rpc); /* invalidates json_str */
I do like the fact that the rpc "owns" the storage associated with the response. In the current KVS api, some returned values have to be freed, others not. That aspect at least would be an improvement. And of course this makes async programming possible should you want to issue the get and do other things before the result arrives.
from flux-core.
I agree that the rpc object ownership of storage is a great improvement. I think this is a really nice development. What you have above is a huge usability improvement, if you want my opinion.
If you call kvs_get_json
on an rpc object not created by kvs_request
, will you get EINVAL?
It might be nice if rpc objects had a type associated so you could do
error ("kvs_get_json() called on rpc object of type %s\n", flux_rpc_type (rpc))
I dunno, maybe that isn't particularly useful, but I like this idea of lightweight extensible type system for rpc objects....
from flux-core.
OK, thanks for the encouragement. I'll think a bit more along these lines then.
Good idea giving flux_rpc_t
objects a type. I was also thinking about adding a way to store opaque data in there with a destructor so that complex value types like kvsdir_t
could be allocated internally in the get function and returned to the caller, but still destroyed by flux_rpc_destroy()
.
from flux-core.
Revisiting the API rework, building on futures and jansson style varargs interfaces. Consider this a straw man proposal for put and get interfaces, feedback welcome.
put for single key (commit implied):
flux_future_t *flux_kvs_put (flux_t *h, const char *key, const char *json_str);
flux_future_t *flux_kvs_putf (flux_t *h, const char *key, const char *fmt, ...);
// finish with flux_future_get (f, NULL) or if commit sequence number needed:
int flux_kvs_get_seq (flux_future_t *f, int *seq);
put for one or more keys (atomic transaction, with fence capability):
flux_future_t *flux_kvs_txopen (flux_t *h);
int flux_kvs_txput (flux_future_t *f, const char *key, const char *json_str);
int flux_kvs_txputf (flux_future_t *f, const char *key, const char *fmt, ...);
int flux_kvs_txcommit (flux_future_t *f, int flags);
int flux_kvs_txfence (flux_future_t *f, const char *name, int nprocs);
// finish with flux_future_get (f, NULL) or if commit sequence number needed
// use flux_kvs_get_seq() above
get:
flux_future_t *flux_kvs_lookup (flux_t *h, const char *key, int flags);
int flux_kvs_get (flux_future_t *f, const char *key, const char **json_str);
int flux_kvs_getf (flux_future_t *f, const char *key, const char *fmt, ...);
For the get I was thinking of flags that would allow a recursive get in one RPC and then accessors to retrieve individual key/values from the result. Would need interfaces to walk the returned key space also, but I didn't get that far.
from flux-core.
For the atomic transactions, I wonder if this is a case where a new type would work better than overloading the future_t. It is a bit strange to be calling put, commit, and fence on a future, which is a container for a deferred result. If you had the txopen return a flux_kvs_txn_t, then you could put, commit, and fence on this object, and finally close it. Under the covers it would presumably be built on flux_future_t?
from flux-core.
That might be a good idea. It would mean flux_kvs_txcommit()
and flux_kvs_txfence()
(or equivalent) would accept a flux_kvs_txn_t
and return a future, so that flux_future_wait_for()
and flux_future_then()
could be used for synchronization. Yeah, I think I like that (will edit my propsoal when I get in).
from flux-core.
I like @grondo's use of "txn" rather than "tx", and I think that would be good to use in the function names as well. I would like to see an underscore between "txn" in the function name and the following term (e.g. flux_kvs_txn_commit()).
I am not sure that I understand the semantics of doing a fence on an atomic transaction though. Could you elaborate on what you have in mind there?
Also, I assume that other forms of put, such as "_put_int", "_put_double" will also exist? In that case, shouldn't "flux_kvs_put" be named "flux_kvs_put_json"?
from flux-core.
Regarding txn
function naming, I agree. Instead of editing my comment, I will open a new issue so we can iterate in the description and dump the old discussion here that is not really relevant anymore.
I am not sure that I understand the semantics of doing a fence on an atomic transaction though. Could you elaborate on what you have in mind there?
The fence essentially combines nprocs
commits into one transaction. So while the flux_kvs_txn_t
in that case is not the whole transaction, that batch of changes is still an all or nothing proposition, so stll appropriate?
Also, I assume that other forms of put, such as "_put_int", "_put_double" will also exist?
No I think flux_kvs_txn_putf()
would cover that. E.g. to set key foo
to the integer value 42:
flux_kvs_txn_putf (txn, "foo", "i", 42);
// or
flux_kvs_putf (h, "foo", "i", 42);
from flux-core.
Is there anything else left from this discussion to complete? Or can this issue be closed?
from flux-core.
I think we can close it.
from flux-core.
Related Issues (20)
- flux-perilog-run exits silently with failure when one or more ranks are not online HOT 1
- cron: race may cause cron jobs to never run again
- job manager: track average and maximum job wait time HOT 1
- mpi: multiple brokers per node with multiple MPI tasks per node hangs with mvapich2-2.3.7-intel HOT 2
- shell: slow job termination when `exit-on-error` triggered
- rank 0 broker crash on elcapi HOT 4
- Allow --dependency on job name HOT 25
- job-exec: possible scaling issue managing many job shells HOT 37
- shell/oom: error handling is sub par
- do not use `flux_cmd_setcwd()` unless necessary
- shell segfault when evlog kvs commit times out HOT 1
- flux-resource: add `-q, --queue` and/or `-p, --properties` option to `flux resource list` HOT 1
- add output mode in `flux job taskmap` to map hostnames to taskids
- need to clean old resource.eventlogs
- limit cpus seen by hwloc in Flux jobs HOT 18
- libsubprocess: flux_subprocess_read() could return 0 despite not EOF HOT 6
- job-exec: cleanup comments / files / functions of `bulk-exec` implementation HOT 5
- cgroups memory limit does not cover memory allocated by ROCm hipMalloc()
- job-list: received jobspec update before jobspec HOT 8
- write job statistics to guest namespace on instance exit
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from flux-core.