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

Address issues with sync optimization #2157

Merged
merged 1 commit into from
Oct 27, 2021
Merged

Conversation

dralley
Copy link
Contributor

@dralley dralley commented Oct 21, 2021

@dralley dralley marked this pull request as draft October 21, 2021 17:42
@pulpbot
Copy link
Member

pulpbot commented Oct 21, 2021

WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue.

@dralley dralley force-pushed the new-issues branch 4 times, most recently from 1f4c104 to 1d6a521 Compare October 21, 2021 19:45
@dralley dralley force-pushed the new-issues branch 6 times, most recently from b4b7ad9 to 8184030 Compare October 23, 2021 04:22
@goosemania
Copy link
Member

If the main repository had changed and subrepos had not changed, subrepos were being excluded from the publication in mirrored metadata mode.

Subrepos that may have changed were not synced if the main repository had not changed

@goosemania Do both of those sound like real issues or am I missing some context that would lead to one of the cases being valid?

I can't think of a situation when those scenarios are valid use cases. I agree that those are issues and the former is more severe, the latter is less critical, imo.

@dralley dralley force-pushed the new-issues branch 4 times, most recently from 9000e6d to e6ac88d Compare October 25, 2021 22:08
@dralley dralley marked this pull request as ready for review October 25, 2021 22:09
)
except ClientResponseError as exc:
if is_subrepo(directory) and exc.status == 404:
log.warning("Unable to sync sub-repo '{}' from treeinfo.".format(directory))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does i18n work, exactly? If it doesn't translate dynamic arguments then it would be safe to add it here.

result = get_repomd_file(remote, url)
repomd_path = result.path
repomd = cr.Repomd(repomd_path)
repomd_checksum = get_sha256(repomd_path)
Copy link
Contributor Author

@dralley dralley Oct 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either Ina or Tanya suggested doing this in the last PR, and combined with the refactoring it does let us ditch this code, which is great: https://github.com/pulp/pulp_rpm/pull/2157/files#diff-fb406f31027edc1c5b65abe7635bbd2830a7cc05c24a487344d7649017d14628L488-L494

@dralley dralley removed the request for review from goosemania October 26, 2021 13:44
@dralley
Copy link
Contributor Author

dralley commented Oct 26, 2021

@jlsherrill Heads up. Back during the meeting about mirrored metadata, we had discussed whether it should be possible for mirrored-metadata publications to be re-created even if the repositories themselves haven't changed, and no new repository version is created. And I think we agreed to do that for 7.0 / 7.1? (according to the document).

We can actually do that for 3.16.1. Let me know if this would break Katello because we could back off on that change, but if it's beneficial and wouldn't break anything we can keep it.

What happens if mirror=True in 3.14 and 3.15 is:

  • If optimize=False OR optimize=True and the sync can't be optimized because some option changed - a new publication will be created ONLY IF a new repository version was created for the main repo. If it was not, no new publication.

  • If optimize=True and the main repo (the one where the .treeinfo is located) is the same as the last sync, nothing happens. No new sync happens, no new publication.

What happens in 3.16.1 if mirror=True / sync_policy=mirror_complete will be:

  • If optimize=False, or if ANY of the repos (main repo + any .treeinfo repos) have changed since the last sync, all of the syncs will happen, and a new publication will be created regardless of whether a new repository version was created for the main repo.

  • If optimize=True and ALL of the repos (main repo + any .treeinfo subrepos) are the same as the last sync, nothing happens. No new sync happens, no new publication.

@jlsherrill
Copy link
Contributor

that all sounds good to me and should 'just work' with katello. I don't forsee any changes needed

@dralley dralley requested a review from ipanova October 27, 2021 14:17
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.

5 participants