nats-io / nats-architecture-and-design

Architecture and Design Docs
Apache License 2.0
177 stars 20 forks source link

Handle unsigned 64 bit server values. #17

Closed scottf closed 1 year ago

scottf commented 2 years ago

Overview

Ensure client handles unsigned 64 bit server values.

The behavior is documented in ADR-1.

Clients and Tools

Other Tasks

Client authors please update with your progress. If you open issues in your own repositories as a result of this request, please link them to this one by pasting the issue URL in a comment or main issue description.

ripienaar commented 2 years ago

I've added type hints for int, uint64, int32 and time.Duration to the schemas via https://github.com/nats-io/jsm.go/pull/298

scottf commented 2 years ago

At the current time, these values are uint64

json object fields

message meta data

i.e. $JS.ACK.test-stream.test-consumer.4.5.6.1605139610113260007

Index Description
4 number of delivered messages
5 stream sequence
6 consumer sequence

publish options / expectations

aricart commented 2 years ago

in some environments it will not be possible to do this (ie, unsigned is not supported, or the range of the number will overflow native max integer values.

scottf commented 2 years ago

Java is a good example where uint64 is not supported. We considered to make our own uint64 object, but since this would not have been a language primitive and since the int64 has quite a large value, we did not make changes and will have to deal with the consequences if some installation ends up with more than 9,223,372,036,854,775,807 messages we'll worry about it then. @aricart Are there any languages that you are concerned about that do not even support int64? Target hardware that might not?

aricart commented 2 years ago

The javascript stack is spec'ed at 2^53 - 1, on internal testing it seems to work with other things, but unsigned is not one of the options in that environment, so some values could appear negative. To be honest, the sequence is derived from a string (subject) so perhaps the thing to address is to just deal with these values as strings. The only wrinkle is any client trying to understand a sequence numerically.

bruth commented 1 year ago

@aricart I am sure you know this, but just wanted to call out BigInt for the sake of this issue.

aricart commented 1 year ago

@aricart I am sure you know this, but just wanted to call out BigInt for the sake of this issue.

of course, it just requires an API change in all the models, and any value that is reported in JSON would likely require a custom JSON parsing library.

Since a single file is unlikely to hold these counts, it may be more useful to deal with smaller numbers that have some other sequence identifier - In the end the question is whether the smaller platforms require these extra transformations, or do we just need a different strategy.

bruth commented 1 year ago

I was unaware of that you could augment the native JSON.parse.

bruth commented 1 year ago

@scottf Given that everything other than the explicit docs have been checked off (I am not sure what would stated in the docs since this is language specific), are you comfortable closing this?

scottf commented 1 year ago

Yes, fine to close.