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

Implement Lease KeepAlive #68

Open
jeffa5 opened this issue Sep 29, 2022 · 6 comments
Open

Implement Lease KeepAlive #68

jeffa5 opened this issue Sep 29, 2022 · 6 comments
Labels
blocked Blocked on another issue enhancement New feature or request M Less than 1 week p1 Should do. Important work but it will not be a blocker for a release.

Comments

@jeffa5
Copy link
Collaborator

jeffa5 commented Sep 29, 2022

This requires streams (similar to watches) to work.

@jeffa5 jeffa5 added enhancement New feature or request M Less than 1 week p1 Should do. Important work but it will not be a blocker for a release. labels Sep 29, 2022
This was referenced Sep 29, 2022
@jeffa5 jeffa5 added the blocked Blocked on another issue label Oct 20, 2022
@jeffa5
Copy link
Collaborator Author

jeffa5 commented Nov 17, 2022

Currently implemented as a unary call so it is possible to refresh the leases, though not ideal.

@heidihoward
Copy link
Member

Is the plan to implement using unary or wait for bidirectional streaming support in CCF?

@jeffa5
Copy link
Collaborator Author

jeffa5 commented Dec 30, 2022

Due to the way bidirectional streaming (in particular the client streaming part) works I think this needs the same patch as for watches in #186. So a unary implementation doesn't seem to work.

@jeffa5
Copy link
Collaborator Author

jeffa5 commented Jan 3, 2023

Note, this works with http1 requests that aren't streaming (when running LSKV in http1 mode), but does not work with the etcd client (or gRPC streaming in general) on http2 as the client streaming logic isn't applied. I think (but haven't tested) that using a http2 client for the non-streaming style request would still work

@heidihoward
Copy link
Member

Am I correct in saying that these are now supported (though a patch to CCF is still required)?

@jeffa5
Copy link
Collaborator Author

jeffa5 commented Aug 31, 2023

I believe it should be (or at least is in theory). I'll test things out once we get the rb maps remove in CCF main and I get a chance to rebase LSKV on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked on another issue enhancement New feature or request M Less than 1 week p1 Should do. Important work but it will not be a blocker for a release.
Projects
None yet
Development

No branches or pull requests

2 participants