Skip to content
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
27c0701
Migrate storage gRPC error codes to tonic::Status
sitaowang1998 Jun 26, 2026
00c3196
Implement the missing storage gRPC services
sitaowang1998 Jun 26, 2026
b41f226
Refactor ready-task builder to take a task-id closure instead of a trait
sitaowang1998 Jun 26, 2026
a1ee868
Merge branch 'main' into storage-grpc-migration
sitaowang1998 Jun 26, 2026
b4fece6
Merge storage-grpc-migration (with main #342) into storage-grpc-services
sitaowang1998 Jun 26, 2026
b98b085
Rename map_inbound_status to inbound_status_to_error to match main's …
sitaowang1998 Jun 26, 2026
b241c34
Rename map_inbound_status and map_liveness_status to match main's sta…
sitaowang1998 Jun 26, 2026
ff31980
Merge storage-grpc-migration (status_to_error rename) into storage-gr…
sitaowang1998 Jun 26, 2026
429fdbb
Merge branch 'storage-grpc-migration' of github.com:sitaowang1998/spi…
sitaowang1998 Jun 26, 2026
25fc42d
Merge origin/main into storage-grpc-migration
sitaowang1998 Jun 27, 2026
72b1021
Merge storage-grpc-migration (with main #358) into storage-grpc-services
sitaowang1998 Jun 27, 2026
c84d324
Update proto.
LinZhihao-723 Jun 28, 2026
9c2f88f
Merge origin/main (tonic 0.14 bump) into storage-grpc-migration
sitaowang1998 Jun 28, 2026
2cf1432
Merge storage-grpc-migration (tonic 0.14 + common.Void responses) int…
sitaowang1998 Jun 28, 2026
896eef6
Remove session id 0 check
sitaowang1998 Jun 28, 2026
c137a3b
Map inbound queue closed to internal error
sitaowang1998 Jun 28, 2026
0bba7ae
Merge branch 'storage-grpc-migration' into storage-grpc-services
sitaowang1998 Jun 28, 2026
2aa8d8e
Map inbound queue closed to internal error on the server
sitaowang1998 Jun 28, 2026
925219a
Merge origin/main into storage-grpc-services
sitaowang1998 Jun 29, 2026
3ec8e32
Fix build_ready_tasks docstring to reference actual lane marker types
sitaowang1998 Jun 29, 2026
a0c8513
Merge branch 'main' into storage-grpc-services
sitaowang1998 Jun 29, 2026
dbf7c7d
Merge branch 'main' into storage-grpc-services
sitaowang1998 Jun 30, 2026
2fee410
Merge branch 'main' into storage-grpc-services
sitaowang1998 Jul 1, 2026
bf84e09
Merge branch 'main' into storage-grpc-services
sitaowang1998 Jul 2, 2026
8a379bb
Fix logging
sitaowang1998 Jul 3, 2026
74e467f
Fix docstring
sitaowang1998 Jul 3, 2026
7a6f42d
Remove test
sitaowang1998 Jul 3, 2026
edcb47f
Check off .
LinZhihao-723 Jul 4, 2026
d943a94
Add a flag to control whether to fail the service in .
LinZhihao-723 Jul 4, 2026
8fc265d
Done with this PR.
LinZhihao-723 Jul 5, 2026
923c820
Done.
LinZhihao-723 Jul 5, 2026
10555b0
Make comments consistent.
LinZhihao-723 Jul 5, 2026
019e168
Revert comments.
LinZhihao-723 Jul 5, 2026
c170cc2
Revert mod visibility.
LinZhihao-723 Jul 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions components/spider-proto-rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub mod error;
pub mod id;
pub mod io;
pub mod job;
pub mod scheduler_registration;
pub mod unpack;

#[allow(clippy::all, clippy::nursery, clippy::pedantic)]
Expand Down
40 changes: 40 additions & 0 deletions components/spider-proto-rust/src/scheduler_registration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//! Conversions between protobuf scheduler messages and their Spider core representations.

use spider_core::types::scheduler::RegisteredScheduler;

use crate::storage;

impl From<RegisteredScheduler> for storage::Scheduler {
fn from(scheduler: RegisteredScheduler) -> Self {
Self {
scheduler_id: scheduler.id.get(),
ip_address: scheduler.ip_address.to_string(),
port: u32::from(scheduler.port),
}
}
}

#[cfg(test)]
mod tests {
use std::net::IpAddr;

use spider_core::types::{id::SchedulerId, scheduler::RegisteredScheduler};

use crate::storage;

#[test]
fn registered_scheduler_to_protocol_carries_id_ip_and_port() {
const SCHEDULER_ID: SchedulerId = SchedulerId::from(42);
const PORT: u16 = 5678;

let scheduler = storage::Scheduler::from(RegisteredScheduler {
id: SCHEDULER_ID,
ip_address: IpAddr::V4("127.0.0.1".parse().expect("valid IP")),
port: PORT,
});

assert_eq!(scheduler.scheduler_id, SCHEDULER_ID.get());
assert_eq!(scheduler.ip_address, "127.0.0.1");
assert_eq!(scheduler.port, u32::from(PORT));
}
}
101 changes: 101 additions & 0 deletions components/spider-proto-rust/src/unpack/storage.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! [`RequestUnpack`] implementations for `storage.proto` requests.

use std::{net::IpAddr, time::Duration};

use spider_core::types::id::{
ExecutionManagerId,
JobId,
Expand All @@ -8,14 +10,21 @@ use spider_core::types::id::{
TaskId,
TaskInstanceId,
};
use tonic::Code;

use crate::{
storage::{
AddResourceGroupRequest,
ExecutionManagerIdRequest,
JobIdRequest,
PollReadyTasksRequest,
RegisterExecutionManagerRequest,
RegisterJobRequest,
RegisterSchedulerRequest,
RegisterTaskInstanceRequest,
ReportTaskFailureRequest,
ReportTaskSuccessRequest,
VerifyResourceGroupRequest,
},
unpack::{RequestUnpack, UnpackError, common::unpack_task_id},
};
Expand Down Expand Up @@ -134,3 +143,95 @@ impl RequestUnpack for ReportTaskFailureRequest {
))
}
}

/// Unpacks [`AddResourceGroupRequest`] into a tuple containing:
///
/// * The external resource group ID.
/// * The password.
impl RequestUnpack for AddResourceGroupRequest {
type Unpacked = (String, Vec<u8>);

fn unpack(self) -> Result<Self::Unpacked, UnpackError> {
Ok((self.external_resource_group_id, self.password))
}
}

/// Unpacks [`VerifyResourceGroupRequest`] into a tuple containing:
///
/// * The resource group ID.
/// * The password.
impl RequestUnpack for VerifyResourceGroupRequest {
type Unpacked = (ResourceGroupId, Vec<u8>);

fn unpack(self) -> Result<Self::Unpacked, UnpackError> {
Ok((ResourceGroupId::from(self.resource_group_id), self.password))
}
}

/// Unpacks [`RegisterExecutionManagerRequest`] into the execution manager's IP address.
impl RequestUnpack for RegisterExecutionManagerRequest {
type Unpacked = IpAddr;

fn unpack(self) -> Result<Self::Unpacked, UnpackError> {
self.ip_address
.parse::<IpAddr>()
.map_err(|error| invalid_argument(format!("invalid IP address: {error}")))
}
}

/// Unpacks [`ExecutionManagerIdRequest`] into an [`ExecutionManagerId`].
impl RequestUnpack for ExecutionManagerIdRequest {
type Unpacked = ExecutionManagerId;

fn unpack(self) -> Result<Self::Unpacked, UnpackError> {
Ok(ExecutionManagerId::from(self.execution_manager_id))
}
}

/// Unpacks [`RegisterSchedulerRequest`] into a tuple containing:
///
/// * The scheduler IP address.
/// * The scheduler port.
impl RequestUnpack for RegisterSchedulerRequest {
type Unpacked = (IpAddr, u16);

fn unpack(self) -> Result<Self::Unpacked, UnpackError> {
let ip_address = self
.ip_address
.parse::<IpAddr>()
.map_err(|error| invalid_argument(format!("invalid IP address: {error}")))?;
let port = u16::try_from(self.port)
.map_err(|_| invalid_argument(format!("port does not fit in `u16`: {}", self.port)))?;
Ok((ip_address, port))
Comment on lines +203 to +205

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant files first
git ls-files \
  components/spider-proto-rust/src/unpack/storage.rs \
  components/spider-storage/src/grpc.rs \
  components/spider-proto-rust/src/scheduler_registration.rs \
  | sed 's/^/FILE: /'

echo
echo "== storage.rs around the port unpacking =="
sed -n '180,220p' components/spider-proto-rust/src/unpack/storage.rs

echo
echo "== grpc.rs scheduler registration/storage flow =="
sed -n '1,240p' components/spider-storage/src/grpc.rs

echo
echo "== scheduler_registration.rs serialization =="
sed -n '1,220p' components/spider-proto-rust/src/scheduler_registration.rs

echo
echo "== search for port validation / zero handling =="
rg -n "port == 0|must be in 1..=65535|INVALID_ARGUMENT|invalid_argument\\(" components/spider-proto-rust components/spider-storage -S

echo
echo "== protobuf definition for RegisterSchedulerRequest =="
rg -n "message RegisterSchedulerRequest|port" components -g '*.proto' -S

Repository: y-scope/spider

Length of output: 14237


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== RegisterSchedulerRequest definition =="
sed -n '168,186p' components/spider-proto/storage/storage.proto

echo
echo "== related scheduler message(s) in storage.proto =="
sed -n '186,210p' components/spider-proto/storage/storage.proto

echo
echo "== search for any explicit allowance of port 0 / ephemeral ports =="
rg -n "port 0|ephemeral|zero.*port|port must|1..=65535|65535|scheduler.*port" components -S

Repository: y-scope/spider

Length of output: 3589


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== register_scheduler implementation =="
sed -n '640,720p' components/spider-storage/src/state/service.rs

echo
echo "== db protocol docs for register_scheduler =="
sed -n '430,470p' components/spider-storage/src/db/protocol.rs

echo
echo "== db implementation search for scheduler port handling =="
rg -n "register_scheduler\\(|scheduler.*port|port.*scheduler" components/spider-storage/src -S

Repository: y-scope/spider

Length of output: 4969


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== mariadb register_scheduler implementation =="
sed -n '580,660p' components/spider-storage/src/db/mariadb.rs

echo
echo "== scheduler table/schema search =="
rg -n "scheduler.*table|port.*u16|port.*INT|scheduler.*port|CREATE TABLE.*scheduler|scheduler_id" components/spider-storage/src -S

echo
echo "== tests around scheduler registration semantics =="
sed -n '980,1055p' components/spider-storage/tests/mariadb_test.rs

Repository: y-scope/spider

Length of output: 8046


Reject port 0 in scheduler registration.
The unpacker only checks that the value fits in u16, so 0 still reaches storage and gets advertised back unchanged. If 0 is not intended by the protocol, return INVALID_ARGUMENT here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/spider-proto-rust/src/unpack/storage.rs` around lines 203 - 205,
Reject port 0 during unpacking in `Storage::unpack` so invalid scheduler
registrations fail early instead of being stored and advertised. Update the
existing `u16::try_from(self.port)` validation to also return `invalid_argument`
when `self.port` is zero, while keeping the current fit-in-`u16` check for
`port` intact. Use the `Storage::unpack` path and the `invalid_argument` helper
as the key places to adjust.

}
}

/// Unpacks [`PollReadyTasksRequest`] into a tuple containing:
///
/// * The maximum number of entries to return.
/// * The maximum duration to block waiting for entries.
impl RequestUnpack for PollReadyTasksRequest {
type Unpacked = (usize, Duration);

fn unpack(self) -> Result<Self::Unpacked, UnpackError> {
let max_items = usize::try_from(self.max_items).map_err(|_| {
invalid_argument(format!(
"max_items does not fit in `usize`: {}",
self.max_items
))
})?;
Ok((max_items, Duration::from_millis(self.wait_ms)))
}
}

/// Builds an [`UnpackError`] carrying [`Code::InvalidArgument`] and the given message.
///
/// # Returns
///
/// An [`UnpackError`] whose [`Code`] is [`Code::InvalidArgument`] and whose message is `message`.
const fn invalid_argument(message: String) -> UnpackError {
UnpackError {
code: Code::InvalidArgument,
message,
}
}
7 changes: 0 additions & 7 deletions components/spider-scheduler/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,6 @@ pub enum StorageClientError {
#[error("job not found: {0:?}")]
JobNotFound(JobId),

/// The scheduler's storage session is stale.
#[error("stale storage session: {storage_session:?}")]
StaleSession {
/// Storage's current session ID.
storage_session: SessionId,
},

/// The storage server returned an invalid input error.
#[error("invalid storage request: {0}")]
InvalidInput(String),
Expand Down
Loading
Loading