[MGNLHOOK-307] Random problem reading environments on module startup Created: 15/Dec/22  Updated: 01/Feb/23  Resolved: 01/Feb/23

Status: Closed
Project: Magnolia Webhooks
Component/s: None
Affects Version/s: 2.0.0
Fix Version/s: None

Type: Bug Priority: Neutral
Reporter: Javier Benito Assignee: Milan Divilek
Resolution: Fixed Votes: 0
Labels: None
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: 3h Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Issue Links:
Problem/Incident
Relates
relates to MGNLHOOK-309 Make REST client environment aware on... Closed
Sub-Tasks:
Key
Summary
Type
Status
Assignee
MGNLHOOK-314 Implement Sub-task Completed Milan Divilek  
MGNLHOOK-315 Review Sub-task Completed Javier Benito  
MGNLHOOK-316 piQA Sub-task Completed Javier Benito  
MGNLHOOK-317 QA Sub-task Completed Oanh Thai Hoang  
Team: DeveloperX
Date of First Response:
Epic Link: Webhook on SaaS
Sprint: DevX 28, DevX 29
Story Points: 5

 Description   

Symptom

Webhook events are not triggered on environments different than main, even if the definitions appear on Definitions App.

Cause

Sometimes on SaaS webapp (staging), after instance is restarted, this code:

environmentsClient
                    .getEnvironmentNames()
                    .forEach(environmentId -> EnvironmentContextPerThread.runInEnvironmentContext(environmentId, webhookDefinitionRegistry::onUpdate)); 

on Webhooks module startup, is only executed for main environment, but all environments are shown on Environment Switcher and Definitions app shows properly all webhook definition files by environment.

As other environments than main, are loaded asynchronously, when module starts up, definitions are not there and subscribers can not be created for webhooks. But later, those definitions are finally loaded, and appear properly on Definitions App (but subscribers still does not exist, because there were no events on webhooks telling that the definitions were loaded, so subscribers weren't created).

 



 Comments   
Comment by Milan Divilek [ 20/Jan/23 ]

Tested Solutions

Problem

I tested 3 possible solutions (Event via EventBus, Callback, subscriber lazyloading), but all have problem with "MgnlContext is not set" exception when info.magnolia.webhooks.registry.WebhookDefinitionEventHandler#createSubscriberFor creates info.magnolia.webhooks.events.core.EventSubscriber because it's running in different thread where MgnlContext is not set.
This can be workaround by using SystemContext for creating info.magnolia.webhooks.events.core.EventSubscriber. This should be safe as we are not manipulating with any data.

"environment definitions loaded" callback

Unfortunately info.magnolia.config.source.ConfigurationSource doesn't support any callback and {EnvironmentBoundConfigurationSource}} is build via info.magnolia.config.plugin.S3ConfigurationSourcePlugin. This means for using callback we would need to do several API changes in info.magnolia.config.source.ConfigurationSource , info.magnolia.config.source.ConfigurationSourcePlugin, etc. Even it doesn't seems right to have defined callback for Environment related stuff in interfaces which doesn't know nothing about Environments.

Using EventBus and firing event when loading environment is complete.

Firing EnvironmentConfigurationLoadedEvent when environment is loaded here in #whenComplete clause. Disadvantage of this solution is that we are trying to minimalisite using of EventBus.

Init subscribers when first webhook event is fired (lazyloading)

Init info.magnolia.webhooks.events.core.EventSubscriber first time when info.magnolia.webhooks.events.core.EventSubscribers is called. This suffers also from the "MgnlContext is not set" problem, because webhook events are also triggered in different thread.

Main issue

Main issue here is in CLOUD-742, where we changed starting Environments to async. This may cause that that the environment/definitions are not ready when module starts. All the tested solutions above fixes the webhook module, but in case other modules needs defintions to be available at the start up, there's going to be same issue(which is hard to find).

Comment by Christopher Zimmermann [ 23/Jan/23 ]

Regarding the "Main Issue" above:

Is there something special about Webhooks that makes this issue more relevant for them? Or put another waay - is there a reason this issue has been reported against webhooks vs another definition type - such as restEndpoints? Is there another definition type that "needs definitions to be available at the startup"? or conversely - Why does webhooks need the definitions available at startup?

Concretely, which other definitions might still cause problems? All of them? For example:

  • templates
  • contentTypes
  • apps
  • restEndpoionts
  • ...
Comment by Christopher Zimmermann [ 30/Jan/23 ]

Thanks. OK. Well my 2cts would be that webhooks needing to fire during system startup is an edgecase and I would prefer not to support it at all if it would make the system more stable or easier to maintain. 
But we don't need to change anything about that now - ie mdivilek can continue with current fix. (Unless you folks want to discuss it further jsimak jbenito )

Comment by Javier Benito [ 30/Jan/23 ]

I'd say the current solution is ok, is working now.

Generated at Mon Feb 12 10:02:58 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.