[ADDED] Exposes Bytes from stream info in KeyValueStatus

[jnmoyne]

Currently, in order to get the size in bytes of a bucket people are resorting to getting the stream info directly adding themselves a hard coded "KV_" prefix to the bucket name which is not great and not future proof. This exposes Bytes() which is already present in s.nfo in KeyValueBucketStatus.

[ripienaar]

LGTM. Also update ADR pls

[piotrpio]

LGTM

[jnmoyne]

ADR updated (nats-io/nats-architecture-and-design#159)
Someone that can please do the merge for this PR, thanks!

[kozlovic]

@jnmoyne Merged.

[aricart]

note that object store exposes this as size - likely this should be the same for KV, as these things have similar terms.

[jnmoyne]

Personally I prefer Byte (which matches StreamInfo) because size is an overloaded term and can be interpreted two ways (is it the number of keys that have values set? Is it the number of records (which is difference since we can have more than one value per key)? Is it the data size? Is it in bytes or kb? At least with Bytes there's no way to mis-interpret it.

If anything I'd say object store should be changed to Bytes to match this and stream info rather than the other way around (IMHO)

[aricart]

(but this is not about the stream, but the materialized views. In all cases bytes, length, size will be an issue, which will require them to go to the doc. Also - on the JavaScript clients I exposed the underlying stream info, wondering if that is also a good practice for all clients on the buckets (KV or ObjectStore) as it reduce the need to add API to telegraph the stream info information.

[ripienaar]

Go allows you to get the underlying stream info - but it should not be the happy path. We should not forget that backend portability is still a concern to keep in mind for these so the core KV API should provide the things most people need, but with escape hatches for really weird requirements where one might want to do backend specific behaviors.

[jnmoyne]

My interpretation of the KeyValueStatus interface with the BackingStore() function is that it's a provision to be able to support something else besides JetStream as the backing store in the future (maybe I'm reading it wrong, though) and therefore the reason the Stream info is not exposed from KeyValueStatus (though obviously if you know the naming convention for the underlying KV bucket streams you can still get the stream info directly from it (which is what some people were doing)).

[ripienaar]

Yes, correct that’s the behaviour I am referring to.

So I think the status interface needs to have a way to get the size without accessing the stream info. Definitely seems like a oversight on my part when designing that status interface.

[jnmoyne]

Right, which is what this PR does for the Go client lib or are you thinking of something else?

I was also wondering if the status interface should also include something to return the number of keys for which there is a value stored in the bucket?

[ripienaar]

I am just confirming I agree with the PR :)

And specifically pointing out that not exposing stream info as a designed in feature and we really should avoid it for the happy path in response to the JS comments from Alberto.

keys perhaps helpful but we should maybe take a look at etcd and vault APIs to see if they expose it? The current interface was heavily built around the common feature of those (see end of ADR)