config: add rpctimeout #86
Conversation
config.go
Outdated
| @@ -29,6 +29,9 @@ type lndConfig struct { | |||
| // MacaroonName is the name of the macaroon in macaroon dir to use. | |||
| MacaroonName string `long:"macaroonname" description:"The name of our macaroon in macaroon dir to use."` | |||
|
|
|||
| // RPCTimeout is the timeout for rpc calls to lnd in seconds. | |||
| RPCTimeout int `long:"rpctimeout" description:"The timeout for rpc calls to lnd in seconds"` | |||
There was a problem hiding this comment.
nit: this can be time.Duration directly instead of int, then the default can be 30 * time.Second.
There was a problem hiding this comment.
Thank you so much!
If I change this to time.Duration that would require the user to specify a unit like: 60s. Is that OK? Or is it better to abstract this from the user so they only have to enter an int?
There was a problem hiding this comment.
Yes, I think that's okay. Perhaps we should add a hint to the description, in lnd we use a text like this: Valid time units are {s, m, h}.
There was a problem hiding this comment.
Cool, that makes sense. Thanks for the reference to lnd. I've pushed the update.
There was a problem hiding this comment.
Thanks! Just need to run make fmt, then this looks good.
wdstorer-bg
force-pushed
the
BG-52292-add-rpctimeout
branch
from
f868bdb
to
eba4e5e
Compare
wdstorer-bg
force-pushed
the
BG-52292-add-rpctimeout
branch
from
eba4e5e
to
85302f0
Compare
This PR adds an option to set the lndclient RPCTimeout when initializing lndclient. This gives lndmon users the option to increase the timeout beyond the default 30 seconds in cases where the metrics take longer to return.
Note: If not specified, this argument defaults to 30 seconds, which is the lndclient default.
Additional context:
We use lndmon for our lnd nodes running in Kubernetes and get frequent timeouts due to latency specific to nodes running with a PostgreSQL backend. As such, it often takes longer than 30 seconds for lndmon to gather metrics resulting in our lndmon pods frequently going into a crashloop status. In issue:78 it was suggested that this could be addressed, on the lndmon side, by increasing the lndclient timeout. This seems to be a good workaround for us so I wanted to present this PR for consideration. Thank you!