KVS: need flux_kvs_move() or flux_kvs_txn_move()

[garlick]

The job manager needs a way to move a job from the active to inactive directories. Currently that would be a combination of a snapshot lookup, a put to the new key, and an unlink of the old key. An example exists in cmd_move() in the flux-kvs command.

The job manager could simply chain futures for these operations (lookup as one RPC, put + unlink + commit as a second one), but I thought I'd open this issue to consider whether it's a useful function to expose in the API, and which way to implement is best:

1. as a composite future (e.g. hiding lookup + commit inside)
2. as a native operation that could be combined with other operations in a transaction

I think 2) would be most useful, but I'm not sure how difficult it would be to implement on the KVS module side.

[garlick]

On 2), I guess it would be two native operations that could appear sequentially in a transaction to represent a move: something like link to crate a new reference from the namespace, followed by the existing unlink on the old reference.

This link is a bit different from UNIX link(2) where you get two names for the same dynamic content. Here the links are just duplicate hash references, so one can change going forward without affecting the other. Maybe that argues for a different name like snapshot?

If link / snapshot could be allowed to cross namespaces, it could also be used for "reaping" a private namespace when a job is made inactive.

[chu11]

If I'm thinking this through correctly a link / snapshot is still just a lookup & put in a transaction, and the move transaction would be the link / snapshot & an unlink.

We already can unlink in a transaction, so that's done. We have mechanisms in place for kvs transactions to lookup missing references (i.e. the root dirref is not yet loaded locally). In the past, we only had to do lookups for missing references within the "tree" of what we were modifying/changing. But now we would have to lookup missing references just to figure out what we need to link to. So it does slow transactions down by adding this extra lookups in the middle. I don't think it's impossible to add, would just be a little extra work plus have to carefully figure out corner cases (i.e. link a val vs a valref vs a symlink).

[garlick]

Could it just always copy a directory entry as is, without checking type? Seems like that would work in all cases.

And actually maybe copy is the right name for the new operation?

[chu11]

Could it just always copy a directory entry as is, without checking type? Seems like that would work in all cases.

Ahh you're right, can just copy the entry.

And actually maybe copy is the right name for the new operation?

If we want support a native copy, then we probably have to consider both shallow & deep copies. The latter being more expensive obviously and probably not what we'd want to use in a move operation.

[garlick]

If we want support a native copy, then we probably have to consider both shallow & deep copies. The latter being more expensive obviously and probably not what we'd want to use in a move operation.

I think actually copying a dirent of any type is equivalent to a deep copy. See cmd_copy() and cmd_move() in flux-kvs.c. Or am I misunderstanding what you mean by deep copy?

[chu11]

I think actually copying a dirent of any type is equivalent to a deep copy.

Right now in treeobj.c shallow and deep copies are different for treeobj dir objects.

[garlick]

Oh sorry I might have a shallow (so to speak) understanding of commit processing at this point. I just meant the end result of copying a dirent (shallow from the copy_treeobj() point of view), would be sufficient to make it look like all its contents have been copied.

[chu11]

I see now, a shallow copy is sufficient b/c it is "deep" from the user perspective looking at the KVS. I'm sure I got confused too :-)

[chu11]

It just occurred to me, having a native copy or move in the KVS module might be difficult given the potential to cross namespaces. For example, doing a flux kvs move ns:FOO/foo ns:BAR/bar

Right now we have a rule that kvs txns can only occur within a single namespace.

[garlick]

Well, we could stick to the restriction. Would that make it more doable?

[chu11]

it definitely makes it more doable. Effectively I have to add a lookup() in the middle of the normal kvs transaction processing and I think it can be done semi-easily. After that it's two kvs modifications instead of one (a "put" followed by a "unlink"), but the code is basically all there already.

Other main thing is right now all kvs transactions are effectively "puts" (regardless if it's a put of a val or dir, that's all encoded in the treeobj). So we'd have to add a "transaction type" kind of field to each transaction operation, so the kvs module can determine it's a "put", "copy", "move" or what not. And obviously for "copy" / "move" we need to add an additional key field for the destination. I don't think the transactions are in any RFC so I don't think we need to update an RFC for this, although maybe we should?

[chu11]

it's probably also worth mentioning that we would want flux kvs move and flux kvs copy to stick with its current implementation, since we do not want it to be restricted to a single namespace. A flux_kvs_txn_copy/move() would only for situations we want to stick a bunch of copies/moves in with other transactions.

[garlick]

it definitely makes it more doable. Effectively I have to add a lookup() in the middle of the normal kvs transaction processing and I think it can be done semi-easily. After that it's two kvs modifications instead of one (a "put" followed by a "unlink"), but the code is basically all there already.

Actually I think since an unlink could be added to a transaction externally, we'd just want to support "copy" (user calls flux_kvs_txn_copy() then flux_kvs_txn_unlink() then commits once).

It seems like this is not that easy to get done within the KVS module, and maybe we should just go with the composite future implementation instead (implementing it within libkvs)? At least for now?

[garlick]

@chu11 were you already working on the libkvs implementations of move and copy?

I was playing around with the composite futures and got something working which I'm fine with tossing out if your'e already going on this. LMK, otherwise I'll go ahead and do the work to turn my toy into something better.

[chu11]

haven't started yet as I've been working on #1651.