Bug 1879126 - no-op repo sync leads to real-op repo publish in certain situations
Summary: no-op repo sync leads to real-op repo publish in certain situations
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: Pulp
Version: 6.7.0
Hardware: x86_64
OS: Linux
medium
high
Target Milestone: 6.9.0
Assignee: satellite6-bugs
QA Contact: Lai
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-09-15 13:47 UTC by Pavel Moravec
Modified: 2024-03-25 16:29 UTC (History)
11 users (show)

Fixed In Version: pulp-2.21.4
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1899310 (view as bug list)
Environment:
Last Closed: 2021-04-21 13:17:46 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
pulp-server-2.21.0.4-2.HOTFIXRHBZ1879126.el7sat.noarch.rpm (784.20 KB, application/x-rpm)
2020-11-04 22:20 UTC, Mike McCune
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Pulp Redmine 7526 0 Normal CLOSED - CURRENTRELEASE Publish is operational when the only change in config is "force_full" setting 2020-11-02 16:08:24 UTC
Red Hat Product Errata RHSA-2021:1313 0 None None None 2021-04-21 13:18:02 UTC

Description Pavel Moravec 2020-09-15 13:47:05 UTC
Description of problem:
There are few scenarios leading to a redundant "real-op" publish(*) of a repo in pulp. 

(recall: each repo sync automatically triggers publish of the repo; if nothing has changed, the publish is no-op - BUT the decision "nothing has changed" wrongly detects "a change" even if the sync didnt update the repo at all)

Particular reproducer:
- republish repository metadata of a repo
  - this will forcefully publish the repo
- then invoke optimised repo sync
  - this wont sync anything new, BUT the repo will be re-published, redundantly

(alternate reproducer: do a full repo sync, followed by optimised/normal repo sync - again, the 2nd (no-op) sync triggers "real-op" publish)

The cause is here: https://github.com/pulp/pulp/blob/2-master/server/pulp/server/controllers/repository.py#L1201

Here, we compare previous and current publish override options. So if a new repo sync does not change anything but it was called with different (even empty) override config, the repo *will be re-published - since same_override=False and https://github.com/pulp/pulp/blob/2-master/server/pulp/server/controllers/repository.py#L1215 evals to False .


As a consequence, redundant repo is re-published (which can also trigger redundant sync of the repo to Capsule, after all).


Version-Release number of selected component (if applicable):
Sat 6.7
pulp-server-2.21.0.1-1.el7sat.noarch


How reproducible:
100%


Steps to Reproduce:
1. Have a fully synced repo in Sat
2. Do either action:
  - invoke Complete Sync
  - or Republish Repository metadata
3. Check the repo was re-published
4. Invoke a regular sync
5. Check the repo was republished

Standalone pulp minimalistic reproducer: call repo publish once with override_config\":{\"force_full\":true}, then without it (or with force_full:false) - even the 2nd publish will be a "real op".


Actual results:
5. Repo was republished


Expected results:
5. Repo isnt republished


Additional info:
I understand there can be legit situations where diff between previous and current override_config *must* result in re-publish. I just dont know them and the current logic is simply too strict..

Comment 1 Pavel Moravec 2020-09-17 14:44:25 UTC
Some minimalistic patch to ignore _changes_ in force_full override config:


--- a/pulp/server/controllers/repository.py
+++ b/pulp/server/controllers/repository.py
@@ -1160,13 +1160,17 @@ def check_publish(repo_obj, dist_id, dis
         skip_for_predistributor = published_after_predistributor or not \
             predistributor_last_published
 
-    same_override = dist.last_override_config == config_override
-    if not same_override:
+    if not (dist.last_override_config == config_override):
         # Use raw pymongo not to fire the signal hander
         model.Distributor.objects(
             repo_id=repo_obj.repo_id,
             distributor_id=dist_id).update(set__last_override_config=config_override)
 
+    # do not compare force_full from config override - this particular difference itself does not
+    # determine the need of repo publish
+    config_override = {k: v for k, v in config_override.items() if k != 'force_full'}
+    dist.last_override_config = {k: v for k, v in dist.last_override_config.items() if k != 'force_full'}
+    same_override = dist.last_override_config == config_override
     # Check if content has not changed since last publish and a predistributor is not defined.
     unchanged_content_and_no_predistributor = last_published and not last_updated and \
         not units_removed and not predistributor_id


(a review from pulp devels is welcomed, though..)

Comment 2 Tanya Tereshchenko 2020-09-18 06:43:21 UTC
Pavel, thanks for the patch. It looks reasonable to me.
Feel free to open the upstream PR. I'll file the upstream bug.

Comment 3 pulp-infra@redhat.com 2020-09-18 07:05:41 UTC
The Pulp upstream bug status is at NEW. Updating the external tracker on this bug.

Comment 4 pulp-infra@redhat.com 2020-09-18 07:05:42 UTC
The Pulp upstream bug priority is at Normal. Updating the external tracker on this bug.

Comment 5 Pavel Moravec 2020-09-29 20:16:26 UTC
(In reply to Tanya Tereshchenko from comment #2)
> Pavel, thanks for the patch. It looks reasonable to me.
> Feel free to open the upstream PR. I'll file the upstream bug.

Here you are :)

https://github.com/pulp/pulp/pull/3999


(majority of redundant repo publishes were in my case really due to config_override altering from {} to {force_full=False})

Comment 6 pulp-infra@redhat.com 2020-10-22 19:06:18 UTC
The Pulp upstream bug status is at POST. Updating the external tracker on this bug.

Comment 7 pulp-infra@redhat.com 2020-10-26 13:06:45 UTC
The Pulp upstream bug status is at MODIFIED. Updating the external tracker on this bug.

Comment 8 pulp-infra@redhat.com 2020-10-26 14:07:41 UTC
All upstream Pulp bugs are at MODIFIED+. Moving this bug to POST.

Comment 9 pulp-infra@redhat.com 2020-11-02 16:08:24 UTC
The Pulp upstream bug status is at CLOSED - CURRENTRELEASE. Updating the external tracker on this bug.

Comment 10 Mike McCune 2020-11-04 22:19:28 UTC
*** Satellite 6.7.4 and 6.7.5 Hotfix Available ***

1) Download pulp-server-2.21.0.4-2.HOTFIXRHBZ1879126.el7sat.noarch.rpm from this bugzilla to your Satellite

2) stop services:

satellite-maintain service stop

3) Install:

rpm -Uvh pulp-server-2.21.0.4-2.HOTFIXRHBZ1879126.el7sat.noarch.rpm

4) restart:

satellite-maintain service start

5) resume operations

Comment 11 Mike McCune 2020-11-04 22:20:26 UTC
Created attachment 1726745 [details]
pulp-server-2.21.0.4-2.HOTFIXRHBZ1879126.el7sat.noarch.rpm

Comment 14 Lai 2021-01-05 17:02:28 UTC
Steps to retest

1. Upload manifest
2. Enabled rhel7 repo (content -> repositories)
3. Sync repo
4. Once repo is sync, resync it
5. Content -> product -> rhel7 product -> rhel7 repo -> select action -> advanced sync.  choose Complete sync and sync
6. Once sync is completed, do a regular sync

Expected:
3. Should take a long time
4. Should take seconds
5. Should take a long time
6. Should take seconds

Actual
3. Regular sync took 46 minutes
4. Took 26 seconds
5. Complete sync took 22 minutes
6. Took 26 seconds

Originally, after the complete sync took place and resyncing happens, it took a long time.  After the fix, the syncing took seconds to complete which is what step 6 indicates.

Verified in 6.9.0_07

Comment 17 errata-xmlrpc 2021-04-21 13:17:46 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: Satellite 6.9 Release), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2021:1313


Note You need to log in before you can comment on or make changes to this bug.