sifive / wake

The SiFive wake build tool
Other
86 stars 28 forks source link

rsc: Consider job before blindly caching #1626

Closed V-FEXrt closed 1 month ago

V-FEXrt commented 1 month ago

There are a few reasons do have a precheck before pushing a job to the cache

This PR implement those reasons into a route that the client should check before pushing a new job to the cache

V-FEXrt commented 1 month ago

We won't be able to record how many jobs were too short to be included or how many were duplicates without some sort of breadcrumb. However for the too short exit condition we would end up spamming the log since all of those jobs will no longer be cached

We could maybe a few more columns to the job history table. Then we'd have hits, misses, denied, conflicted, load_shed or something along those lines. We already have the hash here so it doesn't cost much (besides slightly more load and extra disk) to track

Finally in the high load case we are probably doing the right thing by not logging exactly when we shed a request just that we are very likely too at the moment.

Yeah I want to avoid too much extra work when under load. I'm actually somewhat tempted to temper the warning even further since if we are under load that print is likely be triggered a very large amount. Tempted to do something like

if shed_chance > 0.5 && gen_bool(0.1) { LOG }

which would have a 10% chance of showing the the "heavy load" log. Another improvement would be to add a dashboard graph for "chance to shed load"

So I think the tracing summary is I would want to see an info/warn trace on the CONFLICT status return.

On it

colinschmidt commented 1 month ago

DB tracking for these exit conditions sounds great and shouldn't be too expensive 🤞 Yeah only showing the load shedding message sometimes sounds good to me.