Bug 1581415 - [RFE] Prevent running concurrent Capsule Sync against the same Capsule [NEEDINFO]
Summary: [RFE] Prevent running concurrent Capsule Sync against the same Capsule
Keywords:
Status: NEW
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: Capsule - Content
Version: 6.3.1
Hardware: x86_64
OS: Linux
high
high
Target Milestone: Unspecified
Assignee: satellite6-bugs
QA Contact: Satellite QE Team
URL:
Whiteboard:
: 1627478 1956261 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-05-22 17:30 UTC by Pavel Moravec
Modified: 2023-10-07 00:32 UTC (History)
15 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Target Upstream Version:
Embargoed:
bkearney: needinfo? (mmccune)


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Foreman Issue Tracker 24593 0 Normal New Prevent running concurrent Capsule Sync against the same Capsule 2021-01-19 13:09:25 UTC
Red Hat Issue Tracker SAT-19089 0 None None None 2023-07-21 13:13:27 UTC

Description Pavel Moravec 2018-05-22 17:30:38 UTC
Description of problem:
There are really no locks preventing to run more Capsule Sync tasks against the same Capsule. That is very dangerous, as running this many times (i.e. due to an operator mistake) on a Satellite with hundreds or thousands of repos, the level of concurrency in dynflow (and also on the Capsule to some extend) boils the server(s).

(theoretically concurrent capsule sync might affect each other, I guess, so another reason to prevent it - apart of the fact it is ridiculous)


Version-Release number of selected component (if applicable):
any incl. 6.3.1


How reproducible:
100%


Steps to Reproduce:
1. Have an external Capsule
2. 
for i in 1 2 3 4 5; do
  hammer capsule content synchronize --id=2 --async
done
3. Wait a minute, check tasks running


Actual results:
3. shows multiple CapsuleSync tasks running against the same Capsule.


Expected results:
2. 2nd and further CapsuleSync request should failed with "Required lock already taken" like error.
3. No concurrent CapsuleSync tasks against the same Capsule.


Additional info:

Comment 2 Adam Ruzicka 2018-08-13 11:29:49 UTC
Do we want to strictly limit concurrency to "one sync per capsule" or lift the limitation a bit to "one sync per capsule-repository pair" in case of Repository::CapsuleSync and "one sync per capsule-content view pair" in case of ContentView::CapsuleSync?

Comment 3 Adam Ruzicka 2018-08-13 11:36:40 UTC
Created redmine issue http://projects.theforeman.org/issues/24593 from this bug

Comment 4 Pavel Moravec 2018-08-20 07:16:50 UTC
(In reply to Adam Ruzicka from comment #2)
> Do we want to strictly limit concurrency to "one sync per capsule" or lift
> the limitation a bit to "one sync per capsule-repository pair" in case of
> Repository::CapsuleSync and "one sync per capsule-content view pair" in case
> of ContentView::CapsuleSync?

Very good question, as some Caps repo sync can be triggered automatically while some another manually. I dont have (strong) opinion how we should behave in situations like:

(A) a CV repo (or Library repo) sync was automatically created and is in planning/running phase, AND user invokes _full_ Caps sync (such that even the repo sync shall be re-synced) - skipping the user-invoked sync would not fullfill the user's will to fully sync Caps at the time of request

(B) (full) Caps sync is running (at various state wrt. some repo sync dynflow step status), AND Sat triggers the repo sync due to CV publish/promote or repo sync from CDN just finished - skipping the automatically triggered repo sync would mean new content wont get to the Capsule in some bad interleaving of actions

Also, skipping such tasks with an error "lock acquired by task .." might be confusing for end user ("why my full Caps sync failed? I tried it multiple times and there was always some background task preventing my will") - if we decide to skip such tasks, I vote for some another (non-error) statement.


What about preventing multiple (full or per-repo granularity) Caps sync invoked *just* manually (either via WebUI, hammer or API directly)? Since:

- Caps repo sync invoked automatically (by repo publish and a Caps in Library LE, or by CV publish/promote) should not be limited in either case (and these tasks are rarely concurrently running either way)

- an attempt to prevent automatically triggered and user-triggered Caps sync tasks hits problems above

- so just user-triggered Caps sync tasks do matter

Here I am not sure if we can (or plan to have the feature) to manually sync just a repo to a Caps.

Comment 5 Adam Ruzicka 2018-09-11 09:45:13 UTC
*** Bug 1627478 has been marked as a duplicate of this bug. ***

Comment 8 Bryan Kearney 2019-12-03 16:34:36 UTC
The Satellite Team is attempting to provide an accurate backlog of bugzilla requests which we feel will be resolved in the next few releases. We do not believe this bugzilla will meet that criteria, and have plans to close it out in 1 month. This is not a reflection on the validity of the request, but a reflection of the many priorities for the product. If you have any concerns about this, feel free to contact Red Hat Technical Support or your account team. If we do not hear from you, we will close this bug out. Thank you.

Comment 9 Pavel Moravec 2019-12-12 20:43:27 UTC
Isn't this low-hanging fruit, easy to pick? Likewise there is a task lock on a given Content View to prevent concurrent tasks on the CV, adding a task lock on Capsule object will resolve this bugzilla - that sounds easy, no?

Comment 10 Pavel Moravec 2019-12-22 15:45:57 UTC
It should be enough to comment out

https://github.com/Katello/katello/blob/master/app/lib/actions/katello/capsule_content/sync.rb#L5-L7

such that default :all is used per

https://github.com/theforeman/foreman-tasks/blob/master/app/lib/actions/entry_action.rb#L18

. Then, read and write locks are acquired against SmartProxy of given id and multiple Capsule sync attempts fail.

However, this prevents running *any* concurrent content sync to the same Capsule. Assume 2 CVs publish/promote trigger Actions::Katello::CapsuleContent::Sync for mutually exclusive content - my patch would prevent one of those two tasks :(


And I am afraid there is no way to have a more fined lock /o\ .

Comment 11 Bryan Kearney 2020-01-15 20:30:15 UTC
Thank you for your interest in Satellite 6. We have evaluated this request, and while we recognize that it is a valid request, we do not expect this to be implemented in the product in the foreseeable future. This is due to other priorities for the product, and not a reflection on the request itself. We are therefore closing this out as WONTFIX. If you have any concerns about this, please do not reopen. Instead, feel free to contact Red Hat Technical Support. Thank you.

Comment 12 Bryan Kearney 2020-01-15 20:30:17 UTC
Thank you for your interest in Satellite 6. We have evaluated this request, and while we recognize that it is a valid request, we do not expect this to be implemented in the product in the foreseeable future. This is due to other priorities for the product, and not a reflection on the request itself. We are therefore closing this out as WONTFIX. If you have any concerns about this, please do not reopen. Instead, feel free to contact Red Hat Technical Support. Thank you.

Comment 13 Pavel Moravec 2021-06-16 10:28:06 UTC
I am reopening this BZ for possible reconsideration of fixing it.

My idea is to improve the Improvement https://bugzilla.redhat.com/show_bug.cgi?id=sat-smartsync where we track on Satellite the times when a sync of a repo to Capsule was triggered and when it completed.

Can't we extend the logic of:

*At capsule sync time*, if a history event exists in the table for a given repo and smart proxy that 'finished', do not schedule the sync.

such that if we see that (for given repo and Capsule):
- started_at is not empty AND finished_at is empty (or older than started_at)
- started_at is newer than when the repo was published on the Satellite (how to check this? an idea at the bottom)

then we know some sync request for this repo+Caps is pending on the most recent content of the repo on Satellite. Thus, we can skip the repo sync now.

The first condition means a pending sync request for this repo, the 2nd condition means the potential new sync will not sync anything newer than what the pending sync request will do


Two concerns:
1) how to check when a repo was published the last on Satellite? Maybe we store such data somewhere but I cant find it. If not, we can extend the katello_smart_proxy_sync_history table by adding records for internal proxy, to keep this data here.

2) can't be there some scenario that would ignore a valid request of sync? E.g. what if one triggers a Caps sync and cancels the task - won't be this table left in "started_at filled, finished_at empty" status? Can't be there some other scenario?



Apart of the two concerns, I think the request can be implemented in a very simple way and prevent repetitive customers' concerns (today we saw another "caps sync spammer" case..).


Bryan, could you please re-evaluate / assess my proposal?

Comment 15 Mike McCune 2021-07-13 21:54:48 UTC
Upon review of our valid but aging backlog the Satellite Team has concluded that this Bugzilla does not meet the criteria for a resolution in the near term, and are planning to close in a month. This message may be a repeat of a previous update and the bug is again being considered to be closed. If you have any concerns about this, please contact your Red Hat Account team.  Thank you.

Comment 24 Daniel Alley 2022-08-30 03:35:18 UTC
*** Bug 1956261 has been marked as a duplicate of this bug. ***


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