Allow code to customize JS script timeout #7353
Open
+55
−11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bbimber
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
As you might know, JS trigger scripts default to a 60 second timeout. This is almost always more than enough, and if something takes more than 60 seconds might signal you should look at performance. I have some EHR-related ETLs that sync a lot of data between servers. They are largely stable and fine within that 60 second window, but we can get periods when one or more of these bumps against that window. I spent some time trying to improve that and gave up. Some of the caching that can be triggered in DemographicsService makes it difficult to avoid, and it's not user-facing. This is a longstanding issue I've been living with, but I am hoping to find a solution.
This PR proposes to allow code to customize that timeout. I propose we use the existing 'extraContext' mechanism. If 'scriptTimeout' is provided as an integer, RhinoService should use that value instead of 60. As a precaution, I set the MAX_ALLOWABLE_TIMEOUT as 300 (this is somewhat arbitrary), to prevent some code from going on forever. This means that the client or server-side code that makes the request can provide "scriptTimeout: XX", which should be respected.
I don't see a way for the JS trigger scripts themselves to modify extraContext to change the scriptTimeout themselves. This would be convenient, but is probably by design. While I cant write this myself anymore, it would be nice is the DataIntegration module ETL XML had a property for 'scriptTimeout', and then ETL would poke that into extraContext when it inserts/updates/deletes rows. ETL already pokes in "isETL" to extraContext, so I think the scope of that change ought to be rather small.
There's a related PR in TestAutomation to illustrate how this could work, at least from the client.
Does this concept seem acceptable? If so, I can expand some test cases. I'm happy to discuss other approaches to this.