Skip to content

Commit 8582d49

Browse files
committed
feat(nodedb): wire lease refcount into SharedState and pgwire routing
SharedState gains a lease_refcount field (Arc<LeaseRefCount>) and acquire_plan_lease_scope, which walks a DescriptorVersionSet, increments the refcount for each descriptor, pays the raft acquire round-trip only for first-holder entries, and returns a QueryLeaseScope the handler holds through execute. The pgwire sql_exec path acquires a scope for both cache hits and fresh plans so drain sees held leases regardless of whether the plan was replanned or served from cache. The forwarding logic is extracted from routing/mod.rs into a new routing/forward.rs submodule. forward.rs owns remote_leader_for_tasks and forward_sql, with the latter wrapped in retry_on_not_leader to handle transient leader elections between the routing decision and the RPC dispatch. retry_on_not_leader is added to the retry module with the same budget and backoff shape as retry_on_schema_change so client-visible latency is bounded across both retry surfaces. The session store and pgwire types are updated to thread the DescriptorVersionSet-based plan cache API through to the handler.
1 parent 7525705 commit 8582d49

8 files changed

Lines changed: 481 additions & 145 deletions

File tree

nodedb/src/control/server/pgwire/handler/retry.rs

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,48 @@ where
7878
}))
7979
}
8080

81+
/// Run `op` up to `MAX_ATTEMPTS` times. Retries only on
82+
/// `Error::NotLeader`. Any other error is returned immediately
83+
/// on the first attempt. Same retry budget and backoff shape as
84+
/// [`retry_on_schema_change`] so client-observable latency is
85+
/// bounded across both retry surfaces.
86+
pub async fn retry_on_not_leader<F, Fut, T>(mut op: F) -> Result<T, Error>
87+
where
88+
F: FnMut() -> Fut,
89+
Fut: std::future::Future<Output = Result<T, Error>>,
90+
{
91+
let mut last_err: Option<Error> = None;
92+
for attempt in 0..MAX_ATTEMPTS {
93+
match op().await {
94+
Ok(value) => return Ok(value),
95+
Err(Error::NotLeader {
96+
vshard_id,
97+
leader_node,
98+
leader_addr,
99+
}) => {
100+
tracing::debug!(
101+
attempt,
102+
%leader_node,
103+
%leader_addr,
104+
"pgwire: retrying forward after NotLeader"
105+
);
106+
last_err = Some(Error::NotLeader {
107+
vshard_id,
108+
leader_node,
109+
leader_addr,
110+
});
111+
if attempt + 1 < MAX_ATTEMPTS {
112+
tokio::time::sleep(BACKOFFS[attempt]).await;
113+
}
114+
}
115+
Err(other) => return Err(other),
116+
}
117+
}
118+
Err(last_err.unwrap_or_else(|| Error::PlanError {
119+
detail: "retry_on_not_leader: no attempts recorded".into(),
120+
}))
121+
}
122+
81123
#[cfg(test)]
82124
mod tests {
83125
use super::*;
@@ -131,6 +173,74 @@ mod tests {
131173
assert_eq!(calls.load(Ordering::SeqCst), MAX_ATTEMPTS);
132174
}
133175

176+
#[tokio::test]
177+
async fn not_leader_first_attempt_success() {
178+
let calls = AtomicUsize::new(0);
179+
let result: Result<i32, Error> = retry_on_not_leader(|| {
180+
let c = calls.fetch_add(1, Ordering::SeqCst);
181+
async move { Ok(c as i32) }
182+
})
183+
.await;
184+
assert_eq!(result.unwrap(), 0);
185+
assert_eq!(calls.load(Ordering::SeqCst), 1);
186+
}
187+
188+
#[tokio::test]
189+
async fn not_leader_retries_then_succeeds() {
190+
let calls = AtomicUsize::new(0);
191+
let result: Result<&str, Error> = retry_on_not_leader(|| {
192+
let n = calls.fetch_add(1, Ordering::SeqCst);
193+
async move {
194+
if n < 2 {
195+
Err(Error::NotLeader {
196+
vshard_id: crate::types::VShardId::new(0),
197+
leader_node: 1,
198+
leader_addr: "127.0.0.1:9000".into(),
199+
})
200+
} else {
201+
Ok("done")
202+
}
203+
}
204+
})
205+
.await;
206+
assert_eq!(result.unwrap(), "done");
207+
assert_eq!(calls.load(Ordering::SeqCst), 3);
208+
}
209+
210+
#[tokio::test]
211+
async fn not_leader_exhausts_budget() {
212+
let calls = AtomicUsize::new(0);
213+
let result: Result<(), Error> = retry_on_not_leader(|| {
214+
calls.fetch_add(1, Ordering::SeqCst);
215+
async move {
216+
Err(Error::NotLeader {
217+
vshard_id: crate::types::VShardId::new(0),
218+
leader_node: 1,
219+
leader_addr: "127.0.0.1:9000".into(),
220+
})
221+
}
222+
})
223+
.await;
224+
assert!(matches!(result, Err(Error::NotLeader { .. })));
225+
assert_eq!(calls.load(Ordering::SeqCst), MAX_ATTEMPTS);
226+
}
227+
228+
#[tokio::test]
229+
async fn not_leader_skips_non_matching_errors() {
230+
let calls = AtomicUsize::new(0);
231+
let result: Result<(), Error> = retry_on_not_leader(|| {
232+
calls.fetch_add(1, Ordering::SeqCst);
233+
async move {
234+
Err(Error::PlanError {
235+
detail: "syntax".into(),
236+
})
237+
}
238+
})
239+
.await;
240+
assert!(matches!(result, Err(Error::PlanError { .. })));
241+
assert_eq!(calls.load(Ordering::SeqCst), 1);
242+
}
243+
134244
#[tokio::test]
135245
async fn non_retryable_error_surfaces_immediately() {
136246
let calls = AtomicUsize::new(0);
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
//! Cross-node SQL forwarding: leader detection + RPC dispatch.
2+
//!
3+
//! Split out of `routing/mod.rs` to keep that file under the
4+
//! 500-line soft limit and to give the forwarding path its own
5+
//! home as typed leader-forwarding retry logic grows.
6+
//!
7+
//! The forwarding path is taken when:
8+
//!
9+
//! - Every planned task targets a single vShard whose leader is
10+
//! a remote node, AND
11+
//! - The caller's read consistency requires leader execution
12+
//! (Strong) or the local node is not a replica of that vShard.
13+
//!
14+
//! When taken, we send the original SQL text to the remote leader
15+
//! via the existing `ForwardRequest` RPC. The leader's
16+
//! `LocalForwarder` re-plans and executes locally, then ships
17+
//! back the serialized row payloads. This is the pre-gateway
18+
//! pattern (shipping SQL strings instead of physical plans); the
19+
//! gateway rewrite replaces it with `ExecuteRequest` carrying
20+
//! the pre-planned physical task bytes.
21+
22+
use pgwire::api::results::{Response, Tag};
23+
use pgwire::error::{ErrorInfo, PgWireError, PgWireResult};
24+
25+
use crate::control::planner::physical::PhysicalTask;
26+
use crate::types::{ReadConsistency, TenantId};
27+
28+
use super::super::core::NodeDbPgHandler;
29+
use super::super::plan::{PlanKind, payload_to_response};
30+
use super::super::retry::retry_on_not_leader;
31+
32+
impl NodeDbPgHandler {
33+
/// Check if every task targets a single remote leader we
34+
/// should forward to. Returns `None` if any task should run
35+
/// locally, if the tasks fan out across leaders, or if the
36+
/// metadata routing table has no opinion yet.
37+
pub(super) fn remote_leader_for_tasks(
38+
&self,
39+
tasks: &[PhysicalTask],
40+
consistency: ReadConsistency,
41+
) -> Option<u64> {
42+
let routing = self.state.cluster_routing.as_ref()?;
43+
let routing = routing.read().unwrap_or_else(|p| p.into_inner());
44+
let my_node = self.state.node_id;
45+
46+
let mut remote_leader: Option<u64> = None;
47+
48+
for task in tasks {
49+
let vshard_id = task.vshard_id.as_u16();
50+
let group_id = routing.group_for_vshard(vshard_id).ok()?;
51+
let info = routing.group_info(group_id)?;
52+
let leader = info.leader;
53+
54+
if leader == my_node {
55+
return None;
56+
}
57+
if !consistency.requires_leader() && info.members.contains(&my_node) {
58+
return None;
59+
}
60+
if leader == 0 {
61+
return None;
62+
}
63+
64+
match remote_leader {
65+
None => remote_leader = Some(leader),
66+
Some(prev) if prev != leader => return None,
67+
_ => {}
68+
}
69+
}
70+
71+
remote_leader
72+
}
73+
74+
/// Forward a SQL query to a remote leader node via QUIC.
75+
///
76+
/// Wraps the RPC dispatch in `retry_on_not_leader` so a
77+
/// transient leader election between the routing decision
78+
/// and the forwarded RPC auto-retries up to 3 times with
79+
/// 50ms / 100ms / 200ms backoff. After the retry budget the
80+
/// error surfaces as `Error::NotLeader` which
81+
/// `error_to_sqlstate` maps to a typed Postgres error code.
82+
pub(super) async fn forward_sql(
83+
&self,
84+
sql: &str,
85+
tenant_id: TenantId,
86+
leader: u64,
87+
) -> PgWireResult<Vec<Response>> {
88+
let transport = match &self.state.cluster_transport {
89+
Some(t) => t,
90+
None => {
91+
return Err(PgWireError::UserError(Box::new(ErrorInfo::new(
92+
"ERROR".to_owned(),
93+
"55000".to_owned(),
94+
"cluster transport not available".to_owned(),
95+
))));
96+
}
97+
};
98+
99+
let leader_addr = self
100+
.state
101+
.cluster_topology
102+
.as_ref()
103+
.and_then(|t| {
104+
let topo = t.read().unwrap_or_else(|p| p.into_inner());
105+
topo.get_node(leader).map(|n| n.addr.clone())
106+
})
107+
.unwrap_or_else(|| format!("node-{leader}"));
108+
let leader_addr_for_err = leader_addr.clone();
109+
110+
let deadline_ms =
111+
std::time::Duration::from_secs(self.state.tuning.network.default_deadline_secs)
112+
.as_millis() as u64;
113+
114+
let responses: Vec<Response> = retry_on_not_leader(|| async {
115+
let req = nodedb_cluster::rpc_codec::RaftRpc::ForwardRequest(
116+
nodedb_cluster::rpc_codec::ForwardRequest {
117+
sql: sql.to_owned(),
118+
tenant_id: tenant_id.as_u32(),
119+
deadline_remaining_ms: deadline_ms,
120+
trace_id: 0,
121+
},
122+
);
123+
124+
let resp =
125+
transport
126+
.send_rpc(leader, req)
127+
.await
128+
.map_err(|e| crate::Error::NotLeader {
129+
vshard_id: crate::types::VShardId::new(0),
130+
leader_node: leader,
131+
leader_addr: format!("{leader_addr} (rpc error: {e})"),
132+
})?;
133+
134+
match resp {
135+
nodedb_cluster::rpc_codec::RaftRpc::ForwardResponse(fwd) => {
136+
if !fwd.success {
137+
// A "not leader" failure surfaced from the
138+
// remote leader means our topology view is
139+
// stale — bubble it up as a typed NotLeader
140+
// so the retry helper can take another pass.
141+
if fwd.error_message.contains("not leader")
142+
|| fwd.error_message.contains("NotLeader")
143+
{
144+
return Err(crate::Error::NotLeader {
145+
vshard_id: crate::types::VShardId::new(0),
146+
leader_node: leader,
147+
leader_addr: leader_addr.clone(),
148+
});
149+
}
150+
return Err(crate::Error::PlanError {
151+
detail: format!("remote execution failed: {}", fwd.error_message),
152+
});
153+
}
154+
155+
let mut responses = Vec::with_capacity(fwd.payloads.len());
156+
for payload in &fwd.payloads {
157+
responses.push(payload_to_response(payload, PlanKind::MultiRow));
158+
}
159+
if responses.is_empty() {
160+
responses.push(Response::Execution(Tag::new("OK")));
161+
}
162+
Ok::<Vec<Response>, crate::Error>(responses)
163+
}
164+
other => Err(crate::Error::PlanError {
165+
detail: format!("unexpected response from leader: {other:?}"),
166+
}),
167+
}
168+
})
169+
.await
170+
.map_err(|e| {
171+
let (severity, code, message) =
172+
crate::control::server::pgwire::types::error_to_sqlstate(&e);
173+
PgWireError::UserError(Box::new(ErrorInfo::new(
174+
severity.to_owned(),
175+
code.to_owned(),
176+
format!("{message} (forward target: {leader_addr_for_err})"),
177+
)))
178+
})?;
179+
180+
Ok(responses)
181+
}
182+
}

0 commit comments

Comments
 (0)