search

rqlite / helm-charts

Helm charts for rqlite
MIT License
9 stars 0 forks source link

Update default rqlite version to 8.23.0 #19

Closed otoolep closed 8 months ago

otoolep commented 8 months ago

@jtackaberry

jtackaberry commented 8 months ago

Thanks @otoolep !

Looking at the new sync query param for /readyz, it seems fairly safe and reasonable to use this for a startup probe (i.e. not take any traffic initially until /readyz?sync returns 200 just to ensure things have caught up during initial startup. But as an ongoing readiness probe once traffic starts flowing in, it's not clear if this is wise. After all, the client can dictate the read consistency.

What do you think? Maybe you could expand a bit on the use case(s) that motivated this enhancement?

otoolep commented 8 months ago

Yeah, I wouldn't change readiness probes. sync is for specific needs, which folks might have.

I introduced sync while helping users who had large snapshots transferring between nodes, and were having a hard time knowing when the transfer was complete. The change didn't really help, but the work was done, so I went ahead and added it. strict_freshness (new for Read consistency control) ended up being more effective.

https://github.com/rqlite/rqlite/issues/1672 -- contains a lot of the discussion.

jtackaberry commented 8 months ago

Thanks for the link. (Somehow I missed that when skimming the PR.)

Reading (more or less :)) through the discussion I can see a benefit to adding a startup probe that includes the new sync flag, just to make sure a node is fully up-to-date before it begins handling requests. Will work up a PR for that.

otoolep commented 8 months ago

Reading (more or less :)) through the discussion I can see a benefit to adding a startup probe that includes the new sync flag, just to make sure a node is fully up-to-date before it begins handling requests.

Unless that node is going to serve queries with none Read Consistency, and some sort of freshness interval, there isn't much point though, as all reads will go back the Leader anyway. As long as the node is part of the cluster, it can service reads. It doesn't have to be "caught up".

It was with read-only (non-voting) nodes that kicked this whole thing off.

otoolep commented 8 months ago

Honestly, I don't think I'd bother. It just delays the node being "ready", for little reason. Again, unless the user knows what they are doing, and have a special read-only use case, with none consistency.

jtackaberry commented 8 months ago

Honestly, I don't think I'd bother. It just delays the node being "ready", for little reason.

Yeah, I see your point. Fair enough, will leave things as they are. Thanks!