Remove HALT option from MCP as it can result in corruption #2667
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2667 +/- ##
============================================
- Coverage 52.52% 52.40% -0.13%
+ Complexity 5304 5295 -9
============================================
Files 751 751
Lines 30065 30060 -5
Branches 3883 3880 -3
============================================
- Hits 15793 15752 -41
- Misses 12807 12848 +41
+ Partials 1465 1460 -5
Continue to review full report at Codecov.
|
|
Recommend you also set the watchdog in FAM default to -1 (meaning no watchdog, never terminate.) It is possible to have watchdog and halt features as long as you're not breaking a commit in progress -- I advise looking at the current stack of worker threads to see if they are not in a commit somehow, and if not in a commit then allow the halt. It's a bit tricky but ultimately it could prevent other disasters as it was intended to do when I added that feature. Say someone starts something and they want to halt it because they meant to only do a dry run, and they catch it during the analysis phase so no changes are committed, etc. |
…eout in ThrottledTaskRunnerImpl
|
@badvision Forced task.timeout to For now, more pressing issues than trying to inspect stack traces to safely terminate. Hopefully, the processes don't get stuck, and if there are locks in AEM, Support can help understand the cause and remediate. |
|
Isn't this related to https://jackrabbit.apache.org/oak/docs/dos_and_donts.html#Dont_use_Thread.interrupt? |
|
@kwin yup - this PR should prevent FAM from issuing a thread interrupt now; prior the timeout was long (1 hour; we bumped that up a while back) - but apparently ppl still would hit the Halt button occasionally and cause issues. |
|
Yes more or less.
…On Thu, Aug 12, 2021, 10:22 AM Konrad Windszus ***@***.***> wrote:
Isn't this related to
https://jackrabbit.apache.org/oak/docs/dos_and_donts.html#Dont_use_Thread.interrupt
?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2667 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABHH5BW3S3BRUWPLDHZE5RTT4PRK3ANCNFSM5B7SMA3A>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
|
Shouldn't we rather remove all usages of Thread.interrupt() instead of just disabling triggering it via UI? |
|
@davidjgonzalez thanks for removing the halt button. While it's a useful feature, the UI change helps to avoid unnecessary problems with the renovator tool |
|
@kwin yup - should almost be done, might have one more unit test to fix.. working on it. |
bundle/src/main/java/com/adobe/acs/commons/throttling/RequestThrottler.java
Show resolved
Hide resolved
|
@badvision / @kwin - any chance i could get a quick review so i can merge. I think this will be the last one for 5.0.8 |
Removed Halt from MCP process modal as this can result in repo corruption sometimes (per Leo B.)