file locks have the bad side effect of remaining if the process dies in the middle of the generation process. This will also reduce IO on the NFS drive if using multiple web heads.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikeytown2’s picture

Status: Active » Needs review
FileSize
2.43 KB

Should also take care of redirect loops

sandrewj’s picture

Status: Needs review » Needs work

your patch:

1. adds @ on getimagesize to suppress warning messages
2. adjusts relevant comments in code to reflect change in lock system
3. changes lock system to use a database lock

It still probably needs to

1. lock_release after creating the image to prevent default 30 second timeout on parallel request
2. send separate watchdog messages for the case of "received lock but failed to generate images" and "saw a lock in place but after the lock expired there was no derivative image"

mikeytown2’s picture

FileSize
3.17 KB

Thanks for reviewing this. Patch includes 1. & 2.

sandrewj’s picture

I might reword the second error message for clarity to say something like "...but found no generated image from %image using imagecache preset %preset."

After that I will run the patch and hopefully a couple of other people will try it so it can be ready for whenever the next commit comes.

mikeytown2’s picture

Status: Needs work » Needs review
FileSize
3.16 KB

Updated the patch with better wording.

sandrewj’s picture

Status: Needs review » Reviewed & tested by the community

Looks like this works fine.

I don't have a way of generating a series of racing request which are certain to collide in order to test every option, but this does fix the infinite loop problems and holds multiple requests until the image generates.

fizk’s picture

Issue tags: +ImageCache 3

Marking as ImageCache 3.x Todo.