-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fail to send report #195
Comments
I suppose that #164 might fix this. |
@alsuren I've been getting a lot of these errors recently on my MacBook. Is it out of resource or down? |
Last rate limit email I got from upstash was the 10th.
I still haven't set up forwarding for them. Can't do it on my phone though.
The stats reporting should be forked off in the background. If reporting
fails then you should continue on and ignore it. It should not be printing
whether it passed out failed by default.
…On Tue, 14 Mar 2023, 09:54 Jiahao XU, ***@***.***> wrote:
@alsuren <https://github.com/alsuren> I've been getting a lot of these
errors recently on my MacBook.
Can you check the stats server please?
Is it out of resource or down?
—
Reply to this email directly, view it on GitHub
<#195 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB6FN32AI37WPBKPXCDTN3W4AW3FANCNFSM6AAAAAAVYP6KIA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
That's exactly how
It's just a warning from |
universal-apple-darwin is not in the list in Is it a new thing? You can also get the body of the response and print it if you want more info (I'm currently on my phone, but I can guess what it would say) |
Yes, it's a new target we add for Currently the upstream has not added official support yet and it seems that they are not planning to add a new target for this, but I also don't think they will use |
Ah.
I think that binstall should probably not look in the quickinstall archives
for this architecture then. If it is not in `rustup target list` (or
however you spell it) then quickinstall will never have a built package for
it.
Relatedly: I noticed in the stats that binstall often logs requests for a
handful of architectures (normally glibc and musl). Do you try them all in
parallel or each in series (once you failed to download a more preferred
arch)?
…On Wed, 15 Mar 2023, 05:47 Jiahao XU, ***@***.***> wrote:
Yes, it's a new target we add for cargo-binstall.
Currently the upstream has not added official support yet and it seems
that they are not planning to add a new target for this, but I also don't
think they will use universal-apple-darwin for anything else, so we use
that name in cargo-binstall for universal MacOS binaries that includes
both x86_64 and aarch64 binaries.
rust-lang/cargo#8875 <rust-lang/cargo#8875>
—
Reply to this email directly, view it on GitHub
<#195 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB6FNY4RAPD57UNKWZUE6LW4FCXDANCNFSM6AAAAAAVYP6KIA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Got it, I can add this as a special case to our QuickInstall fetcher since this is the only target we invented.
cargo-binstall's resolution process is actually made up of two parts for each crate:
The first process is done in parallel and the second is done in serial. The quickinstall report is done in ::find, which is why binstall logs for both glibc and musl. I could add a new API Fetcher::report_failure that will be called if Fetcher::find returns Ok(false) on the second step. |
That makes sense. It also explains why you blast through your GitHub rate
limit so quickly.
Brain dump of unrelated ideas follows:
A) When the list of requested packages*fetchers is long, it might be
interesting to limit the concurrency a bit, by
* making a list containing the preferred fetcher for all packages first,
followed by the next best fetcher for each package etc
* Pulling from it with bounded concurrency primitive like stream::buffered
https://docs.rs/futures/latest/futures/stream/trait.StreamExt.html#method.buffered
or a semaphore or workerpool
* Picking the first result from the list for each package and dropping the
rest (some of which wouldn't have even been spawned yet, of they are far
enough down the list)
(You would also need a mechanism for a successful fetcher search step for
$package to cancel any lower priority fetchers for that package)
B) if each quickinstall fetcher was run strictly in series then its search
step could be a noop (always return "maybe") and the download step could do
the fetch of the package tarball at that point only (and also report stats
at this point). This would drastically reduce the number of http fetches
you make to GitHub releases, because you don't do any of the HEAD requests,
and in the happy case you only do one GET.
C) I would find it really interesting if we could report which fetcher+arch
binstall used for each package. The new stats server is much easier to
extend than the old one (just add arbitrary query params and they will be
added to the influxdb stats). It might be better to delay stats reporting
until after the find calls have resolved, and then send a single report. We
can design this properly once I have finished building it (no promises
about when that will happen though, because I'm a bit snowed under at the
moment with work and life things).
…On Wed, 15 Mar 2023, 09:00 Jiahao XU, ***@***.***> wrote:
Ah.
I think that binstall should probably not look in the quickinstall archives
for this architecture then. If it is not in rustup target list (or
however you spell it) then quickinstall will never have a built package for
it.
Got it, I can add this as a special case to our QuickInstall fetcher since
this is the only target we invented.
Relatedly: I noticed in the stats that binstall often logs requests for a
handful of architectures (normally glibc and musl). Do you try them all in
parallel or each in series (once you failed to download a more preferred
arch)?
cargo-binstall's resolution process is actually made up of two parts for
each crate:
- Call Fetcher::find on each fetcher for each target, which returns an
AutoAbortJoinHandle, a new type over tokio::task::JoinHandle that cancel
the task on drop, collect the handles into a Vec
- Iterate over the Vec, await on the handle.
If it returns error, propagate.
If it returns Ok(true), then call Fetcher::download and check whether
the archive contains all binaries required (all binaries not gated behind
any feature).
If it contains all the bins required, resolution done.
Otherwise, on Ok(false) or missing bins, proceed the iteration.
The first process is done in parallel and the second is done in serial.
The quickinstall report is done in ::find, which is why binstall logs for
both glibc and musl.
I could add a new API Fetcher::report_failure that will be called if
Fetcher::find returns Ok(false) on the second step.
—
Reply to this email directly, view it on GitHub
<#195 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB6FN54ACQYZ6UESWX6M4LW4FZIXANCNFSM6AAAAAAVYP6KIA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thanks, that's a good advice though I'm not sure how to implement this. Our current solution of using GitHub Restful API already reduces the http requests created for For
We already have this mechanism in place using
With GitHub Restful API client in place, we don't need to run it in serial anymore since one API call to GitHub would return all release artifacts in one http call for the entire crate.
Yes this is exactly what I am going to do. |
Fixed cargo-bins/cargo-quickinstall#195 - Fix `fetchers::QuickInstall`: Stop sending stats for `universal-apple-darwin` since quickinstall only supports targets officially supports by rust. - Only send stats report to quickinstall if the `Fetcher::find` is `.await`ed on This prevents stats report to be sent for cases where the `QuickInstall` fetcher is actually unused, e.g. resolved to `GhCrateMeta` or other `QuickInstall` fetcher with different target. This also reduces amount of http requests created in background. Signed-off-by: Jiahao XU <[email protected]>
The text was updated successfully, but these errors were encountered: