wip crdt spec #316
wip crdt spec #316
Conversation
Signed-off-by: R.I.Pienaar <[email protected]>
Signed-off-by: R.I.Pienaar <[email protected]>
ripienaar
requested review from
wallyqs,
derekcollison,
neilalexander,
bruth and
MauriceVanVeen
Signed-off-by: R.I.Pienaar <[email protected]>
Signed-off-by: R.I.Pienaar <[email protected]>
|
I think integers only unless you feel we should support floats.. |
Seemed like no extra effort to do float so went with it. But, pulling in some comments from the email thread here, we need to limit it to |
|
Actually a decent amount of effort actually to determine to use atoi or atof (essentially).. |
|
I am not liking the 2^53 but if we keep total as a string can be higher yes? |
|
if you do the string yes, the 2^53 is the size from the JSON spec... as a string, libs have a chance to parse as big num or whatever they require. |
so what should we make the top ceiling for protection against overlflow assuming ints - |
|
We could say int64 for now and learn more as we go.. |
|
Or uint64 if all positive? |
sounds complex :) but we can move in that direction if needed, so int64 is a good start |
|
uint64 is going to be parsed incorrectly by some JSON parser... |
|
We could chose to have the value as a string and the client lib parses it however they want. So JS/TS still has a limit but not languages with true 74bit or 128bit numbers. Doesn't JS have a big int thing now too? |
Signed-off-by: R.I.Pienaar <[email protected]>
|
Yeah JS does now have BigInt, it seems to me starting with int64 and then moving to exotic weird things later is a best option. But from the get-go value as string. Updated ADR for that |
Signed-off-by: R.I.Pienaar <[email protected]>
|
Otherwise it requires a custom JSON parser, to convert the numeric value to a string, and then change it. As a string, the flow can be outside of the JSON parsing which imho is better |
Signed-off-by: R.I.Pienaar <[email protected]>
adr/ADR-48.md
Outdated
|
|
||
| * The header holds values like `+1`, `-1` and `-10`, in other words any valid `int64`, if the value fails to parse | ||
| the message is rejected with an error. A value of `0` is valid. | ||
| * When publishing a message to the subject the last value is loaded, the body is parsed, incremented and written |
There was a problem hiding this comment.
Feel like we need to strongly define what "the last value" is, i.e. it must be from the leader only, cannot be direct-get'd from a replica.
Also do we have to think about interactions with Nats-Expected-Last-Sequence, Nats-Expected-Subject-Last-Sequence etc?
There was a problem hiding this comment.
Previous message for the subject - this is done on the server where the message is received so would be the leader right?
We might for now reject those headers just to avoid weird things?
There was a problem hiding this comment.
Have marked those all as rejected and made a small clarification
Signed-off-by: R.I.Pienaar <[email protected]>
Signed-off-by: R.I.Pienaar <[email protected]>
Signed-off-by: R.I.Pienaar <[email protected]>
|
|
||
| It's important that counters can be reset to zero, in a simple standalone counter this is easily done with a subject purge. In a topology where multiple regions contribute to a total a regional reset might require a negative value to be published equal to the current total. | ||
|
|
||
| The preferred method for Reset should be a negative value being published followed by a purge up to the message holding the negative value, with subject purge being used only for entire counter deletes. |
There was a problem hiding this comment.
Maybe we can split the responsibilities here:
- Publishing negative values - a way to reset the counter.
- Purging - a way to clear old values.
Does solve the problem of resetting in more advanced topologies at least.
| * When a message with the header is published to a Stream without the option set the message is rejected with an error | ||
| * When a message with the header is received over a Mirror the message is stored verbatim | ||
| * When a message without the header is received and the previous message is a counter this will be rejected since | ||
| the subject is then treated as being a counter only. This could be very expensive on Streams with many |
There was a problem hiding this comment.
Maybe we can say that Stream with Counters enabled is always counter only to substantialy reduce the cost of those checks?
There was a problem hiding this comment.
Yeah, may be worth starting there, we can always relax later
|
I think this should be the first feature developed for the 2.12, and relased early for preview so we can gather feedback allowing us to improve before the API is released and cannot have breaking changes anymore. Otherwise, we might land in a place where great feature with good design is crippled with small mistakes that could be easily avoided, making it a missed opportunity for a great user experience. |
Very strongly agree with this |
|
So we are now saying do not try to squeeze this as alpha/beta into 2.11? Or are we agreeing put it into 2.11 as beta and 2.12 is when it becomes real? |
|
From writing the ADR I think this is not the trivial easy win we were thinking it is, especially considering sources (the place where this will really shine) we should use the 2.12 cycle - hopefully a 6 months one - to get this right and focus the remaining of our 2.11 time to get Msg TTL right. Both are very big ticket items that we must be sure to get right. So for me, I think we need to pick this up as early / first delivery for 2.12 cycle |
|
I agree with @ripienaar If we have it in early in 2.12 preview release, it will effectively be a preview beta that we can stabilize for the 2.12 release. The end result will be the same (released in 2.12), but without a risk of not being able to improve it or having serious bugs. |
|
What part is hard about sourcing? |
read the section here https://github.com/nats-io/nats-architecture-and-design/blob/74e0f2086be6b36031e76ef5b209d051bab3a521/adr/ADR-48.md#source-based-replicated-counters and about adding sources at runtime |
No description provided.