Versionstamped key index in transaction not written

[ryanworl]

https://github.com/josephg/node-foundationdb/blob/b3b33add2b07c84708ee556aeb60b69e124591f9/lib/transaction.ts#L331-L338

Details here: https://forums.foundationdb.org/t/implementing-versionstamps-in-bindings/250/5

For transactions where more than one versionstamped key is written, bytes 11-12 need to have a LE uint16 written to them indicating the index of the versionstamped operation within the transaction. This allows the two versionstamped keys to be ordered within the transaction. Then two additional bytes need to be added to the format to hold the offset of the versionstamp again.

You can verify this behavior by attempting to write more than one versionstamped key within the same transaction. The versionstamped keys will all be the same.

[ryanworl]

My initial issue is very poorly worded. The actual insertion of data works fine with the current behavior. You just can't differentiate the order of versionstamped keys inserted within the same transaction.

The link to the forum post has the best description of this.

Basically you append a LE uint16 to the key you've already generated so as to be able to differentiate between versionstamped keys within a transaction.

https://forums.foundationdb.org/t/implementing-versionstamps-in-bindings/250/4

Another post from the same thread with a better description.

[josephg]

Oh cool, nice catch. I misunderstood the API documentation and thought that was correct 😅

Should be easy enough to fix - I’ll have a poke tomorrow.

[ryanworl]

👍 The API should also allow for creating a versionstamped key with the same index. The idea mentioned in the thread is if you create a verionstamped key and then need to index that key some way in another structure. Both would need the same versionstamp index value.

i.e.

users/data/VERSIONSTAMP_01 => record A { name = Ryan, username = ryanworl } users/indexes/username/ryanworl/VERSIONSTAMP_01 => ''

In both cases you want the same logical index in the key but have written two different versionstamped keys in the transaction itself.

[ryanworl]

example API suggestion

setVersionstampedKey(prefix: Buffer, suffix: Buffer | null, value: Buffer, index?: number): number

If index is undefined, a number private to the Transaction class is used and incremented. Otherwise, use the provided index.

Both return the index which was used. (in place of current void return type)

[josephg]

Wanna write a PR?

[ryanworl]

Sure, probably will have time this week

On Jun 11, 2018, at 8:06 PM, Seph Gentle notifications@github.com wrote:

Wanna write a PR?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[josephg]

I'm just taking a look at this now. I had a read of the python and java code, and I think I disagree about how it should be implemented.

It looks like the atomic API in both libraries is pretty raw - it doesn't do any packing, or any prepending of a 2 byte code. The actual logic which appends a 2 byte transaction-internal code seems to be specific to tuple encoding.

I'm not quite sure what I want my API to be, but I want to let users use their own versionstamp convention if they aren't using the tuple encoder. I might end up pulling any versionstamp smarts out of the transaction class and put it in the tuple encoder instead. This is more consistent with the other bindings.

[ryanworl]

Having had time to work with FDB a bit more since I raised this issue, I agree 100%

[josephg]

I've implemented this as part of #19. Closing issue for now, though the changes haven't been finalized & published yet