Improve storage access patterns in ReportsProcessed DO #384
Conversation
mendess
requested review from
armfazh,
bhalleycf,
captain-mota,
chris-wood and
cjpatton
as code owners
mendess
force-pushed
the
mendess/improve-mark-aggregated
branch
3 times, most recently
from
8b1345a
to
f2ecdd8
Compare
| @@ -1,6 +1,7 @@ | |||
| # SPDX-License-Identifier: BSD-3-Clause | |||
|
|
|||
| [workspace] | |||
| resolver = "2" | |||
There was a problem hiding this comment.
This is so cargo can properly resolve the feature sets and not pull in tokio when building for wasm (since tokio doesn't support wasm)
There was a problem hiding this comment.
Would you mind adding a comment to clarify?
| @@ -45,26 +43,56 @@ pub(crate) const DURABLE_REPORTS_PROCESSED_MARK_AGGREGATED: &str = | |||
| /// where `<report_id>` is the hex-encoded report ID. | |||
| #[durable_object] | |||
| pub struct ReportsProcessed { | |||
| #[allow(dead_code)] | |||
| state: State, | |||
| env: Env, | |||
| config: DaphneWorkerConfig, | |||
| touched: bool, | |||
| alarmed: bool, | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Please document the meaning of this constant. How did you pick 131,072? Also, is this the best name?
There was a problem hiding this comment.
that's the max amount a value can have inside a Durable Object, I got it from the error message. I'll document
| .into_iter() | ||
| .flatten() | ||
| .collect::<Vec<ReportId>>(); | ||
| let mut seen = self.load_seen_reports().await?; |
There was a problem hiding this comment.
High level question: Isn't this going to get expensive if the number of reports processed gets large? Do we have a sense of how many is too many? Once we reach that point, what should we do at the protocol level?
There was a problem hiding this comment.
I tested for more than 8k reports and it behaved just fine. But perhaps we need to measure what a real workload looks like
There was a problem hiding this comment.
Did you test this in wrangler dev or on Workers?
To avoid creating a DoS vector here, we're going to want to enforce a cap on the number of reports we're willing to store in a single instance. If we overflow this, then we'll need to reject reports for which we aren't tracking replay (with PrepareError "report_dropped"). Note that the "mark aggregated" call back will have to handle the rejection logic.
mendess
force-pushed
the
mendess/improve-mark-aggregated
branch
6 times, most recently
from
a94664e
to
4c904e4
Compare
The format processed reports are now stored in is an array of bytes with the 16 byte wide ids contiguous in memory
mendess
force-pushed
the
mendess/improve-mark-aggregated
branch
from
4c904e4
to
a1dc740
Compare
No description provided.