stolon
stolon copied to clipboard
sorintlab
Missing timeout for libkv based stores
We don't have timeout for libkv based stores(we are using consul) at here. Can we add it? Is there any reason for not having it?
@dineshba libkv doesn't support context and looks unmantained. Please take a look #677
Can we create cancelable Context and cancel it after timeout ? I think this can be done outside libkv. Lemme try and raise a PR.
@dineshba If the underlying libkv operation doesn't support a context we don't have a way to cancel the underlying operation. You could thinkg to execute the libkv inside a goroutine but you don't have a way to cancel that goroutine since it's blocked to the libkv function and will be stale causing all kind of issues.
I agree that we cannot close the connection(it will become a stale connection). I think we can write code in such a way that stale connection will not create problem. What do you think?
On side note: If
libkvis not scalable, can we slowly migrate away from it? May be we can start with consul first.
I think we can write code in such a way that stale connection will not create problem. What do you think?
I'm not sure how this could be done. It will leak file descriptor and perhaps it could work for reads, but writes will end with concurrency issues since a "stale" write could complete after one issued later. I'll avoid this and just do what you proposed below:
On side note: If libkv is not scalable, can we slowly migrate away from it? May be we can start with consul first.
Yes, you can find another better library or reimplement everything directly using the related client like done for etcdv3.
A store timeout command line option has been added in #765. Obviously it'll be honored only by stores that handle it, so it won't work with libkv consul. So the solution is always the same: find another better library or reimplement everything directly using the related client like done for etcdv3.