Skip to content

[wicketd] switch to a watch channel for installinator reports#3950

Merged
sunshowers merged 5 commits into
mainfrom
sunshowers/spr/wicketd-apply-backpressure-to-installinator-reports
Aug 29, 2023
Merged

[wicketd] switch to a watch channel for installinator reports#3950
sunshowers merged 5 commits into
mainfrom
sunshowers/spr/wicketd-apply-backpressure-to-installinator-reports

Conversation

@sunshowers

@sunshowers sunshowers commented Aug 24, 2023

Copy link
Copy Markdown
Contributor

This was originally changed to an unbounded receiver in #3579, but in
the comments to RFD 400 it was pointed out that maybe we shouldn't give
up on backpressure. Switch to a watch channel, since we only care about
the last element anyway.

(This isn't a hugely pressing issue, but I want to include it as a case
study in RFD 400.)

Also treat the receiver being closed as HTTP 410 Gone. We don't do
anything with this at the moment, but could abort installinator on
receiving this signal in the future.

Created using spr 1.3.4
Created using spr 1.3.4
@sunshowers sunshowers changed the title [wicketd] apply backpressure to installinator reports [wicketd] switch to a watch channel for installinator reports Aug 24, 2023
Created using spr 1.3.4
Comment thread wicketd/src/installinator_progress.rs Outdated
(
// Don't set the state to Closed here even if this is a
// terminal update, because we want to keep returning
// ReceiverClosed.

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.

I don't entirely follow this comment, but I'm having a hard time finding where Self::Closed vs Self::ReportsReceived(_) is used or reported - can you point me in the right direction?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've expanded the comment -- hopefully this is clearer.

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.

It is, thank you!

Comment on lines 90 to +105
RunningUpdate::ReportsReceived(sender) => {
slog::debug!(
self.log,
"further report seen for this update ID";
"update_id" => %update_id
);
*update =
RunningUpdate::send_and_next_state(sender, report);
let (new_state, ret) = RunningUpdate::send_and_next_state(
&self.log, sender, report,
);
*update = new_state;
ret
}
RunningUpdate::Closed => {
// The sender has been closed; ignore the report.
*update = RunningUpdate::Closed;
EventReportStatus::Processed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is where the ReportsReceived and Closed statuses are read.

Created using spr 1.3.4
@sunshowers sunshowers requested a review from jgallagher August 28, 2023 23:06
@sunshowers sunshowers merged commit eb2dc91 into main Aug 29, 2023
@sunshowers sunshowers deleted the sunshowers/spr/wicketd-apply-backpressure-to-installinator-reports branch August 29, 2023 18:15
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.

2 participants