Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
AbstractiveNord opened this issue Feb 29, 2024 · 8 comments
Labels
proposal Enhancement idea or proposal

Comments

@AbstractiveNord
Copy link
Contributor

Proposed change

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

Use case

Distributed TTL based locks, leader election systems.

Contribution

Yes.

@AbstractiveNord AbstractiveNord added the proposal Enhancement idea or proposal label Feb 29, 2024
@Jarema
Copy link
Member

Jarema commented Feb 29, 2024

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
Copy link
Contributor Author

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
Copy link
Member

Jarema commented Feb 29, 2024

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
Copy link
Contributor Author

AbstractiveNord commented Feb 29, 2024

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

@AbstractiveNord
Copy link
Contributor Author

AbstractiveNord commented Feb 29, 2024

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
Copy link
Member

Jarema commented Feb 29, 2024

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

@AbstractiveNord
Copy link
Contributor Author

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
Copy link
Member

Jarema commented Mar 1, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Enhancement idea or proposal
Projects
None yet
Development

No branches or pull requests

2 participants