There's a race condition with the Ooyala module that is very easy to run into with short videos or long content forms. If node auto-publishing is enabled, the workflow usually goes something like this:

  1. User starts uploading a video. Drupal gets an embedcode for the video being uploaded.
  2. Node is saved to Drupal.
  3. When the video is processed, Ooyala pings Drupal, and Drupal fetches information for the video.

The problem is if the ping is sent before the node is saved. Drupal doesn't have the embedcode saved, and discards the ping. Instead, we should keep pings around, but have some sort of timeout or retry count. Then, Drupal can see if the node has been saved, and refresh from Ooyala.

This could also be solved by publishing the node automatically when the node is saved. A queue system benefits us in that it also solves a series of other issues including:

  1. Keeping pings in the queue if we can't successfully connect back to Ooyala.
  2. Manually refreshing all content for cases where the site was offline or the network connection was broken.
  3. Handling the loss of an embedcode when Ooyala marks an upload as a duplicate after saving a node (new issue, but I think a queue is a good way to go).

I'm planning on using the D7 queue API and the D6 backport at drupal_queue for the 6.x-1.x version.

CommentFileSizeAuthor
#3 1407196.3-ping-queue-6.x.patch18.45 KBdeviantintegral
#2 1407196.2-ping-queue.patch47.38 KBdeviantintegral
#1 1407196.1-ping-queue.patch40.56 KBdeviantintegral

Comments

deviantintegral’s picture

deviantintegral commented
Assigned: deviantintegral » Unassigned
Status: Active » Needs work
StatusFileSize
new 1407196.1-ping-queue.patch40.56 KB

Here's an initial patch that implements the following changes. Highlights include:

  • Refactoring all ping code into ooyala.ping.inc.
  • Fetch items queued during cron.
  • By default, attempt to fetch videos up to 10 times, and then discard the ping request.

Here's the commit log:

  • 234c658 Issue #1407196: Refactor Ooyala ping functions into their own file.
  • 3cf31e9 Issue #1407196: Standardize variable names to use embed_code.
  • c78fa6b Issue #1407196: Remove legacy D6 cache disabling code.
  • a62ced8 Issue #1407196: Add basic queue insert functions.
  • 0dcd3c6 Issue #1407196: Move all request logging to the ping page callback.
  • 5d020c0 Issue #1407196: Move the cache disable to the ping page function.
  • 570db46 Issue #1407196: Add missing return in _ooyala_ping_fetch_image().
  • a1dec7b Issue #1407196: Refactor ping processing into an explicit process function.
  • be314c8 Issue #1407196: Queue pings when they are recieved and process them with cron.
  • a06be19 Issue #1407196: Fix else statement outside of the while loop.
  • 144e302 Issue #1407196: Fix wrong variable when logging ping requests.
  • 516e0b7 Issue #1407196: Fix processing the same item multiple times and retrying failed items.
  • 2e3a5bd Issue #1407196: Acutally use the passed in $embed_code parameter.
  • 65335b6 Issue #1407196: Fix missing array keys for D7 field array formats.
deviantintegral’s picture

deviantintegral commented
StatusFileSize
new 1407196.2-ping-queue.patch47.38 KB

Here's a new patch that adds the following commits.

  • 2bea06e Issue #1407196: Fix wrong phpdoc for the ooyala_ping_process and insert functions.
  • c7a530f Issue #1407196: Add comments about why we manually run our queue.
  • ead5b81 Issue #1407196: Fix incrementing fetch attempts in the wrong place.
  • d27e8d2 Issue #1407196: Only process a unique embed code once per queue process.

I'm pretty happy with this. I'm going to push these up to the 7.x-1.x branch. A 6.x-1.x patch will follow shortly.

deviantintegral’s picture

deviantintegral commented
Status: Needs work » Fixed
StatusFileSize
new 1407196.3-ping-queue-6.x.patch18.45 KB

Here is the 6.x backport that I've been developing concurrently. This consists of a single commit with all of the changes from the 7.x patch, followed with a commit for drupal_queue support:

  • b03dc0b Issue #1407196: Backport of 7.x-1.x ping queue processing.
  • ee524a4 Issue #1407196: Require the drupal_queue module and include it's files when needed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.