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

Fix marking GitHub-sourced experiment crates as complete #756

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

Mark-Simulacrum
Copy link
Member

When processing GitHub repositories, our list usually (always?) contains a repository without any form of commit hash. Crater agents checkout that repository and, as part of building it, record the commit hash they used. The agent then submits that hash to the db for storage.

When storing the hash, we replace the "crate name" (owner.reponame) with a more specific ID (e.g., owner.reponame.$cratehash). This means that the set of crates we tested has effectively changed at this point from our perspective. Next we store results (previously under the original name, now under the new name) and also update the (previous old, now new) experiment_crates record to mark it complete.

The net effect is that prior to this commit (and likely since ~Aug 31) every GitHub repository has been repeatedly tested by Crater until we eventually hit count(results) >= count(experiment_crates). This is basically just a random point in time though, AFAICT there's no relationship between the set of crates we wanted to test and the set of results we have. One saving factor is there's some amount of fixed point -- if the GitHub repository we test doesn't receive any new commits between attempts to run it, we'll re-test the same code and the old/new IDs will match, letting us mark it as complete. But this is at best a minor improvement, it's not actually a mitigating factor.

As a future TODO, we probably should update the "finish condition" from counting results and experiment_crates and instead use something like "are there any experiment_crates with a status of queued" which makes much more sense.

When processing GitHub repositories, our list usually (always?) contains
a repository without any form of commit hash. Crater agents checkout
that repository and, as part of building it, record the commit hash they
used. The agent then submits that hash to the db for storage.

When storing the hash, we *replace* the "crate name" (owner.reponame)
with a more specific ID (e.g., owner.reponame.$cratehash). This means
that the set of crates we tested has effectively *changed* at this
point from our perspective. Next we store results (previously under the
original name, now under the new name) and also update the (previous
old, now new) experiment_crates record to mark it complete.

The net effect is that prior to this commit (and likely since ~Aug 31)
every GitHub repository has been repeatedly tested by Crater until we
eventually hit count(results) >= count(experiment_crates). This is
basically just a random point in time though, AFAICT there's no
relationship between the set of crates we wanted to test and the set of
results we have. One saving factor is there's some amount of fixed point
-- if the GitHub repository we test doesn't receive any new commits
between attempts to run it, we'll re-test the same code and the old/new
IDs will match, letting us mark it as complete. But this is at best a
minor improvement, it's not actually a mitigating factor.

As a future TODO, we probably should update the "finish condition" from
counting results and experiment_crates and instead use something like
"are there any experiment_crates with a status of queued" which makes
much more sense.
@Mark-Simulacrum Mark-Simulacrum added this pull request to the merge queue Dec 12, 2024
Merged via the queue into rust-lang:master with commit c2a7846 Dec 12, 2024
3 checks passed
@Mark-Simulacrum Mark-Simulacrum deleted the debug-crater branch December 12, 2024 01:58
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.

1 participant