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

publish-timeout not receiving updated index #11253

Closed
ehuss opened this issue Oct 17, 2022 · 5 comments · Fixed by #11255
Closed

publish-timeout not receiving updated index #11253

ehuss opened this issue Oct 17, 2022 · 5 comments · Fixed by #11255
Labels

Comments

@ehuss
Copy link
Contributor

ehuss commented Oct 17, 2022

Problem

I'm trying to test out the publish timeout, and I can't seem to get it to work. I'm running the command:

CARGO_PUBLISH_TIMEOUT=60 cargo publish -Zpublish-timeout

It waits for 60 seconds, and then prints the warning:

warning: timed out waiting for `ehuss-test1` to be in crates.io index

Steps

  1. Run CARGO_PUBLISH_TIMEOUT=60 cargo publish -Zpublish-timeout

Possible Solution(s)

No response

Notes

I did some basic instrumenting, and it doesn't look like cargo is reaching the network to update the index (CARGO_HTTP_DEBUG).

I also monitored the GitHub index, and the package is showing up within a few seconds.

Version

Tried latest nightly cargo 1.66.0-nightly (b332991a5 2022-10-13) and latest master 3ff0443.

@epage
Copy link
Contributor

epage commented Oct 18, 2022

Something seems to be short circuiting. For a couple iterations, the index update puts the source back in updated_sources() but then it stops doing that soon after.

   Uploading epage-publish-test v0.1.1 (/home/epage/src/personal/epage-publish-test)
[src/cargo/ops/registry.rs:407] "iter" = "iter"
[src/cargo/ops/registry.rs:415] config.updated_sources().remove(&source.replaced_source_id()) = true
    Updating crates.io index
     Waiting on `epage-publish-test` to propagate to crates.io index (ctrl-c to wait asynchronously)
[src/cargo/ops/registry.rs:407] "iter" = "iter"
[src/cargo/ops/registry.rs:415] config.updated_sources().remove(&source.replaced_source_id()) = true
[src/cargo/ops/registry.rs:407] "iter" = "iter"
[src/cargo/ops/registry.rs:415] config.updated_sources().remove(&source.replaced_source_id()) = false

Going to keep looking.

@epage
Copy link
Contributor

epage commented Oct 18, 2022

Confirmed there is a cache somewhere causing problems by re-loading the source inside the loop.

@epage
Copy link
Contributor

epage commented Oct 18, 2022

So RemoteRegistry::load is checking self.needs_update which is false and just reuses what is already in-memory.

We are calling RemoteRegistry::invalidate_cache but that doesn't actually invalidate the cache if self.updated.

But nothing sets self.updated = false.. Dropping the check in invalidate_cache makes things work, I just need to figure out what is intended by all of this.

    needs_update: bool, // Does this registry need to be updated?
    updated: bool,      // Has this registry been updated this session?

If we are only allowed to update once per session, then what is the point of invalidate_cache? And this also seems redundant with config.updated_sources()

@epage
Copy link
Contributor

epage commented Oct 18, 2022

@arlosi, looks like you implemented the invalidate_cache code in #10064. Am I reading this right that there isn't a way to actually force a registry update remote registries? What was the intention for how this was supposed to work?

@arlosi
Copy link
Contributor

arlosi commented Oct 18, 2022

You should be okay with removing the if !self.updated { check in invalidate_cache. The intent was to prevent multiple updates in a single session, but it's redundant with updated_sources.

Maybe we should completely remove the updated field and rely on updated_sources instead. Something like this: 2c0bd34

Calling invalidate_cache is telling the source that it needs to be fully up-to-date on the next query (can't use an existing git clone or cache files). Without calling invalidate_cache, the queries can return data that's days+ old.

bors added a commit that referenced this issue Oct 20, 2022
fix(publish): Check remote git registry more than once post-publish

With `publish-timeout` on remote git registries, we were checking once and never checking again.  There were 3 layers of guards preventing the cache from being updating
- `needs_update`, handled via `invalidate_cache`
- `config.updated_sources()`,. handled by removing from the HashSet
- `updated`, inaccessible

This change collapses `updated` into `config.updated_sources()`, allowing the cache to be updated

Tested by publishing `epage-publish-test`.  After about 7 registry updates, it succeded.  Before, it just hit the timeout of 60s.  No tests are added as we don't have the plumbing setup to be able to control when the test remote git registry publishes.  All of our tests focus on the remote http registry.

Fixes #11253
@bors bors closed this as completed in dd69674 Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants