Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(spooler): Remove use of legacy project cache #4419

Merged
merged 19 commits into from
Jan 8, 2025

Conversation

iambriccardo
Copy link
Member

@iambriccardo iambriccardo commented Dec 23, 2024

This PR removes the old legacy project cache and implements the forwarding logic directly in the spooler. The new logic works as follows:

  • We still optimistically assume that a new project key pair is ready. (This assumption is intentionally made without the sync check to simplify the buffering code)
  • When peeking, if the key pair is ready, we synchronously load the project states. If at least one state is pending, the pop will not occur, and the project configs will be scheduled for fetching. If both states are available, the envelope will either be forwarded to the processor or dropped, depending on the state.

Follow-up work could include investigating how to reintroduce a bounded buffer between the buffer and the processor to prevent unspooling a large number of envelopes at once, which would accumulate in the processor queue. This option is not strictly necessary due to our memory threshold, which halts unspooling, but it may still be worth exploring.

Closes: #4335

pub own_key: ProjectKey,
pub sampling_key: ProjectKey,
own_key: ProjectKey,
sampling_key: Option<ProjectKey>,
Copy link
Member Author

@iambriccardo iambriccardo Dec 23, 2024

Choose a reason for hiding this comment

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

I decided that I would prefer to encode in the pair the lack of sampling key and rather expose different methods for making it behave like before, where they were both set and the fallback of sampling_key was own_key.

@iambriccardo iambriccardo marked this pull request as ready for review January 7, 2025 08:12
@iambriccardo iambriccardo requested a review from a team as a code owner January 7, 2025 08:12
@iambriccardo iambriccardo requested a review from jjbayer January 7, 2025 08:14
Comment on lines 20 to 26
pub fn own_key(&self) -> ProjectKey {
self.own_key
}

pub fn sampling_key(&self) -> ProjectKey {
self.sampling_key
}
Copy link
Member

Choose a reason for hiding this comment

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

any downside of just keeping the fields public?

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, can revert.

relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
Comment on lines 479 to 481
ProjectState::Pending => {
unreachable!("The own project should not be pending after pop");
}
Copy link
Member

Choose a reason for hiding this comment

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

If we merge this with the above check for pending, this panic shouldn't be necessary

Comment on lines 20 to 26
pub fn own_key(&self) -> ProjectKey {
self.own_key
}

pub fn sampling_key(&self) -> ProjectKey {
self.sampling_key
}
Copy link
Member

Choose a reason for hiding this comment

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

+1

relay-server/src/services/buffer/common.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
@iambriccardo iambriccardo merged commit 5d16ad6 into master Jan 8, 2025
25 checks passed
@iambriccardo iambriccardo deleted the riccardo/ref/remove-project-check branch January 8, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Directly check project availability when creating an envelope stack
3 participants