Opt-in querying of non-leaders
This PR is a first stab at allowing non-leader nodes to serve PREPARE, QUERY, and QUERY_SQL requests. When served by a non-leader, such requests will in general return stale data (QUERY, QUERY_SQL), or fail unhelpfully (PREPARE and QUERY_SQL, see #395). But when these limitations are tolerable, the option to query a non-leader can be helpful to spread load or (when targeting the local node) avoid network round-trips.
Clients must opt into relaxing the current leader checks on a per-connection basis. This is done by repurposing the old, unused flags field on the OPEN request (which non-leaders already accept). We relax the leader checks when the least-significant bit is set in flags; this is backwards-compatible since extant versions of go-dqlite always set this field to 0. The specific relaxations are:
- PREPARE is always allowed. (FINALIZE is also allowed, but it didn't have a leader check in the first place.)
- QUERY and QUERY_SQL are allowed when the statement in question is readonly (recall #477).
Signed-off-by: Cole Miller [email protected]
Codecov Report
:x: Patch coverage is 92.70073% with 10 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.01%. Comparing base (06992a6) to head (384712f).
:warning: Report is 71 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/gateway.c | 84.12% | 2 Missing and 8 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #709 +/- ##
==========================================
+ Coverage 80.95% 81.01% +0.05%
==========================================
Files 196 196
Lines 29186 29287 +101
Branches 4089 4090 +1
==========================================
+ Hits 23627 23726 +99
- Misses 3895 3899 +4
+ Partials 1664 1662 -2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Note to self/reviewer: might be worth discussing whether we should skip in leader__barrier if allow_stale is set but we happen to be the leader. In the current version we do skip in this situation, and I think that's the right choice, but I could be convinced otherwise.
I just realized there's an issue I didn't deal with---if you connect to a node that happens to be the leader and open a DB with with the ALLOW_STALE flag, the connection will still be shut down when the node loses leadership. I'll fix that quickly...
Pushed a new revision incorporating feedback from @marco6---the semantics of the OPEN flag have changed, in particular it now blocks modifications even when the server is currently the leader. This makes the behavior of connections using this flag more predictable and pushes clients to choose the right kind of connection for their use-case. The flag has been renamed from ALLOW_STALE to READONLY to reflect this.
Closing this as without support from RAFT it is unclear what the semantics of the reads would be.