search

nats-io / nats.rs

Rust client for NATS, the cloud native messaging system.
Apache License 2.0
1.07k stars 169 forks source link

KV Update command doesn't return last sequence number in case of fail #1221

Open AbstractiveNord opened 8 months ago

AbstractiveNord commented 8 months ago

Proposed change

Add to error message some metadata, such as last sequence number.

Use case

Distributed TTL based locks, leader election systems.

Contribution

Yes.

Jarema commented 8 months ago

Hey!

Thanks for the proposal.

Server actually returns that information, unfortunately as a part of the string descritpion.

However, I would like to ask - what is the use case? If you're doing CAS and Update failed because of sequence mismatch, meaning that something else updated it in the meantime, you cannot perform another update with fixed sequence but without first getting the current revision content. On the other hand if you don't care about the current state of given key, you can simply use put.

How the returned sequence help here?

AbstractiveNord commented 8 months ago

Recently Jeremy from NATS team released video about Jetstream KV, and mentioned example toy project, leader election system. I've tried to implement that project in Rust, and got two problems.

  1. There is no kv.create() method.
  2. I've simulated create with update method, but I need last number to be sure I am working with latest version.
Jarema commented 8 months ago

The create method was added recently and will be part of next release (which will happen shortly).

You can check the main branch to see how it was implemented in the meantime.

AbstractiveNord commented 8 months ago

Awesome news about create method, thanks a lot, but still, errors formatted with metadata, but doesn't contain metadata fields is problem.

AbstractiveNord commented 8 months ago

Server actually returns that information, unfortunately as a part of the string description.

Wait a little, did I got this correctly. Server doesn't clearly return metadata such as last sequence id, just string formatted with that data?

Jarema commented 8 months ago

Server returns the error code (10071 in this case), with a description wrong last sequence: {seq}.

AbstractiveNord commented 8 months ago

Server returns the error code (10071 in this case), with a description wrong last sequence: {seq}.

Should that behavior reported to server team as problematic? It's kinda weird, cause human readable text is additional traffic, probably better to send error code and related metadata, and build an error string on clients itself. Maybe some flag variable to backwards compatibility?

Jarema commented 8 months ago

It's very rarely that the data returned in error is useful in any way for the clients. Such a change would be a pretty big one, with very doubtful benefit right now considering how rarely the data (beyond the error code) is useful.