I have a Google Spreadsheet script function onFormSubmit(e) that is called by a trigger whenever someone submits my form.
Within the function I create a temporary copy of a template document and search and replace in it based on the submitted form values. Then that temporary copy is converted to a pdf and emailed to several email addresses, and then deleted.
Now I've read that locking may be an issue here, and so I decided to get a LockService.getScriptLock(), and wrap my script in a lock trying first to obtain a lock for 30 seconds lock.waitLock(30000), and then releasing the lock at the end of the script lock.releaseLock() to prevent concurrency issues.
When someone submits a form, the script itself never seems to run for more than 13 seconds.
So in summary the code looks a bit like this:
function onFormSubmit(e) {
// Get a public lock on the script
var lock = LockService.getScriptLock();
try {
lock.waitLock(30000); // Wait for 30 seconds
// copy template, search and replace etc...
lock.releaseLock()
}catch(e) {
Logger.log('Could not obtain lock after 30 seconds.');
MailApp.sendEmail("[email protected]", "Could not obtain lock after 30 seconds.");
}
}
Is there anything else I should be doing? Am I locking too much or too little? I remember in my Java days it was a sin to lock the global application session context for too long; also what about making a copy of the template, and opening the copy of the template for search and replace, does that require some sort of locking?
With the performance timings that you've described, you will probably find that you can get away with the function you've shown. At least, until that day when the processing takes extra long because of forces out of your control.
The basic structure of your lock handling is OK, but you are doing too much inside the try..catch. Note that any exception will be caught, but your handling is limited to a waitlock() timeout exception, so you should avoid any other statements that may generate exceptions in the try block.
Here's how you should restructure your function:
function onFormSubmit(e) {
// Perform any "pre" operations on private
// or non-critical shared resources.
var sheet = e.range.getSheet(); // for example, accessing event object
// Get a public lock on the script
var lock = LockService.getScriptLock(); // Choose appropriate scope
try {
// just attempt to get the lock here - nothing else that may
// throw an exception.
lock.waitLock(30000); // Wait for 30 seconds
}catch(e) {
// Handle lock exception here
Logger.log('Could not obtain lock after 30 seconds.');
MailApp.sendEmail("[email protected]", "Could not obtain lock after 30 seconds.");
}
////// Critical section begins vvvvv
// operate only on shared modifiable data
////// Critical section ends ^^^^^
lock.releaseLock()
// Continue with operations on private
// or non-critical shared resources.
// Ensure the lock is released before exiting.
if (lock.hasLock()) {
throw new Error("Lock violation");
}
else {
return;
}
}
The Lock Service provides a way to fence off a critical section of code. A critical section is where we control access to shared resources that must be stable for reading or writing. The principle here can be summarized thus:
releaseLock() and a hasLock() validation check.I remember in my Java days it was a sin to lock the global application session context for too long...
In Google Apps Script, we don't have that concept; there is no global application session. Scripts may be executed asynchronously, with the possibility of multiple instances for a user as well as instances for other users. In this environment, the concern shifts to locking only at the appropriate scope, to limit the possible impact of the lock.
...what about making a copy of the template, and opening the copy of the template for search and replace, does that require some sort of locking?